Skip to content

feat: Add IPFS backend #481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 29, 2022
Merged

feat: Add IPFS backend #481

merged 1 commit into from
Aug 29, 2022

Conversation

xprazak2
Copy link
Contributor

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

Adds a new backend service provider - IPFS

@Xuanwo Xuanwo changed the title feat: Add IPFS backend (#355) feat: Add IPFS backend Jul 30, 2022
Xuanwo
Xuanwo reviewed Jul 30, 2022
View reviewed changes
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice PR! I left some reviews, PTAL.

And it's required that all PRs should pass cargo fmt and cargo clippy, please make sure they are happy.

@Xuanwo Xuanwo mentioned this pull request Aug 3, 2022
Closed
@Xuanwo
Copy link
Member

Xuanwo commented Aug 11, 2022

@xprazak2 Hi, are you still working on this PR?

@xprazak2 xprazak2 force-pushed the ipfs branch 3 times, most recently from f4b4cd6 to 8015283 Compare August 14, 2022 09:24
@xprazak2
Copy link
Contributor Author

I am still working on it, apologies it is taking me so long. I think I have addressed all the comments except for using new_http_channel, I might need a bit more guidance on it. I was looking at it previously, but I am not sure how to plug it in. I can use Box::pin to pass fut into RequestWriter and skip IpfsWriterFuture but then I get lifetime error:
io-channel

@xprazak2 xprazak2 force-pushed the ipfs branch 2 times, most recently from 7c8d061 to 1062d36 Compare August 14, 2022 09:51
Xuanwo
Xuanwo reviewed Aug 14, 2022
View reviewed changes
@Xuanwo Xuanwo marked this pull request as draft August 15, 2022 08:16
@Xuanwo
Copy link
Member

Xuanwo commented Aug 23, 2022

We have a refactor in #556 to make it easier to implement IPFS backend, PTAL.

@xprazak2 xprazak2 force-pushed the ipfs branch 7 times, most recently from 6dbd323 to 0388e72 Compare August 27, 2022 13:52
@xprazak2
Copy link
Contributor Author

Having access to the reader made it much easier to implement write. I rebased and removed IPFS SDK.

@Xuanwo
Copy link
Member

Xuanwo commented Aug 27, 2022 •
edited
Loading

Having access to the reader made it much easier to implement write. I rebased and removed IPFS SDK.

Please ping me back when you are ready. And leaving comments for any ideas. I'm willing to help.

@xprazak2 xprazak2 force-pushed the ipfs branch 2 times, most recently from 1e55bb5 to a9a505f Compare August 27, 2022 19:21
@xprazak2
Copy link
Contributor Author

@Xuanwo, all green now.

Xuanwo
Xuanwo reviewed Aug 28, 2022
View reviewed changes
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM!

@xprazak2 xprazak2 mentioned this pull request Aug 28, 2022
Closed
@Xuanwo Xuanwo marked this pull request as ready for review August 29, 2022 03:43
@Xuanwo
Copy link
Member

Xuanwo commented Aug 29, 2022

Great, thanks for your PR!

@Xuanwo Xuanwo merged commit 3c918eb into apache:main Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xuanwo Xuanwo Xuanwo left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xprazak2 @Xuanwo