Skip to content
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.

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

docs: add decisions/20240313-try-w3up.md #2518

Merged
merged 9 commits into from
Mar 19, 2024
Merged

docs: add decisions/20240313-try-w3up.md #2518

merged 9 commits into from
Mar 19, 2024

Conversation

gobengo
Copy link
Contributor

@gobengo gobengo commented Mar 13, 2024

Motivation:

Status

  • initial plan drafted
  • update based on feedback

Copy link

cloudflare-workers-and-pages bot commented Mar 13, 2024

Deploying nft-storage with  Cloudflare Pages  Cloudflare Pages

Latest commit: 68ead4f
Status: ✅  Deploy successful!
Preview URL: https://b8906b0c.nft-storage-1at.pages.dev
Branch Preview URL: https://w3s-0007.nft-storage-1at.pages.dev

View logs

@gobengo gobengo requested a review from alanshaw March 13, 2024 23:09
@gobengo gobengo changed the title add decisions/20240313-try-w3up.md docs: add decisions/20240313-try-w3up.md Mar 13, 2024
Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together. This is great

decisions/20240313-try-w3up.md Show resolved Hide resolved
decisions/20240313-try-w3up.md Show resolved Hide resolved

## Alternative Solutions

I don't know of any alternate solutions I can link to that provide a path to servicing nft.storage's web3 storage needs without relying on dagcargo. If you know of some, please add them here.
Copy link
Contributor

Choose a reason for hiding this comment

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

use w3filecoin directly. But that seems already out of the table :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vasco-santos Do you think using w3filecoin directly might be a more promising approach than what we describe here? It can be on the table, I just don't understand what that would entail and pros/cons.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is the proposal for it I wrote a way back and discussed with Alan and Hannah https://hackmd.io/@vasco-santos/Skxt0yd5a/edit

w3filecoin was designed for multiple storefronts, so w3up/others, and my idea was to not go through w3up to avoid mixing the data at that level. And this together with the separate aggregation process for both. The general sense though is that using w3up makes more sense on a perspective on having nft.storage as a user of w3up in the long run. But if the expectation we have is it to be 3 months, making it a client of w3filecoin, while continuing to write to old bucket and so on, would make more sense to me.

But after talking with Alan, Hannah, Reid, we opted on the w3up lane, and that is why I say that it seems out of the table. I generally agree with using w3up because it is simpler if we won't care about splitting aggregation + have nft.storage in the long run as a client of w3up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a section for this alternate solution. Thanks for sharing it and context. 🙏 b3c22cd

Comment on lines 26 to 27
* fully remove reliance on dagcargo
* however, this work may serve as useful input to that. If nft.storage can upload using w3up, then w3up can probably handle the filecoin aggregation via [w3-filecoin](https://github.com/web3-storage/specs/blob/main/w3-filecoin.md). And if we have that, it reduces reliance on dagcargo.
Copy link
Contributor

@vasco-santos vasco-santos Mar 14, 2024

Choose a reason for hiding this comment

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

@gobengo do you mean by this that first iteration is behind a feature flag via ENV VAR + specific accounts? Or do you mean for first iteration we still need dagcargo DB, but not the aggregator given all uploads will go via w3up+w3filecoin path, but we still need DB until second stage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean by this that first iteration is behind a feature flag via ENV VAR + specific accounts?

I mostly meant this. 'fully remove reliance on dagcargo' is a nongoal for first iteration of just making sure we can do at least some uploads via w3up. But I wrote this bullet to say that, while it's a non-goal this sprint, I hope this moves us closer to removing dagcargo by replacing it with w3up.

Assuming this scouting goal goes well, 'fully remove reliance on dagcargo' could be next goal.

Copy link
Contributor

Choose a reason for hiding this comment

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

For me that is fine if the date dagcargo goes off is before sprint ends? worth double checking with @reidlw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me that is fine if the date dagcargo goes off is before sprint ends?

I think the sprint ends on March 27, and my last day of availability is March 22. I'll be back April 1. I don't see a path to dagcargo going off before the sprint ends.

gobengo and others added 3 commits March 14, 2024 09:48
Copy link
Contributor

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

Generally LGTM. Thanks for writing this!

Time crunch issues around staged rollout with feature flag may be a thing, but that can be dealt with outside this doc.

decisions/20240313-try-w3up.md Outdated Show resolved Hide resolved
@gobengo gobengo merged commit 06712ad into main Mar 19, 2024
2 checks passed
@gobengo gobengo deleted the w3s-0007 branch March 19, 2024 22:01

Rollout

- [ ] release modified nft.storage to staging (production ok too) but with web3.storage disabled-by-default via feature switch

Choose a reason for hiding this comment

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

Elizabeth is planning to release a new Website/console which is currently in development by some contractors. We need to understand what she is changing and what the impact on this roll out plan will be.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is an API change, so as soon a new website/console uses same API everything would be fine. But from what I know (or in other words, pieces I put together from all the meetings I attended), the new website/console they are putting together would be a complete new thing that would mean all of this is deprecated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to understand what she is changing and what the impact on this roll out plan will be.

sounds like a good thing for next sprint

alanshaw pushed a commit that referenced this pull request Mar 28, 2024
…gcargoless filecoin deal-making (#2522)

Motivation:
* part of storacha/project-tracking#7
* implement #2518

Note
* many files I modified only because the upgrade to new typescript
compiler or eslint required it
* w3up-client requires node 18 so this drops support for node 16 from
the api package
* I think the client will still work on node 16, but I couldn't find a
way of getting the ci for client to, using node 16, install just the
client package deps and not the api package deps (which tries to install
w3up-client, and then `yarn` errored because that requies node 18)
* w3up-client requires node 18, but everything had been running on node
16
* [ ] if we can't find a way of continuing to use v16, then when this
merges to main, we probably need to go into cloudflare workers settings
and change NODE_VERSION from 16 to 18+

---------

Co-authored-by: Vasco Santos <santos.vasco10@gmail.com>
Co-authored-by: Travis Vachon <travis.vachon@gmail.com>
Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants