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

chore: remove legacy upload codepath #2580

Merged
merged 8 commits into from
Apr 15, 2024
Merged

Conversation

travis
Copy link
Contributor

@travis travis commented Apr 12, 2024

plus feature flag and related tests

travis added 6 commits April 10, 2024 11:30
this seems to fix the build errors I was seeing, and necessitates a small code change
Add a test for fetching filecoin deal info from w3up.

This is definitely not my best work - ideally we'd abstract this w3up mocking logic in a way that makes it less stateful, but I'm hesitant to do that when we will likely not be making many changes here in the future.
this broke existing tests because it was returning w3up deals in all situations
plus feature flag and related tests
abstract w3up mocking a bit and use it in more places
Copy link

cloudflare-workers-and-pages bot commented Apr 12, 2024

Deploying nft-storage with  Cloudflare Pages  Cloudflare Pages

Latest commit: af174d8
Status: ✅  Deploy successful!
Preview URL: https://f0c40a68.nft-storage-1at.pages.dev
Branch Preview URL: https://feat-cleanup-w3up-feature-fl.nft-storage-1at.pages.dev

View logs

@travis travis changed the base branch from main to feat/w3up-deal-info April 12, 2024 22:28
@travis travis marked this pull request as ready for review April 12, 2024 22:28
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.

LGTM

Not sure if you are going for a second PR to remove linkdex, but I think we should/could also remove https://github.com/nftstorage/nft.storage/blob/feat/cleanup-w3up-feature-flag/packages/api/src/routes/nfts-upload.js#L219-L222

I think that once we write to R2 only that will fail, but I am not super aware on how linkdex is built

Base automatically changed from feat/w3up-deal-info to main April 15, 2024 17:58
@travis travis merged commit 88c05a9 into main Apr 15, 2024
6 checks passed
@travis travis deleted the feat/cleanup-w3up-feature-flag branch April 15, 2024 20:49
travis pushed a commit that referenced this pull request Apr 17, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.8.0](api-v4.7.0...api-v4.8.0)
(2024-04-17)


### Features

* show deal info from w3up
([#2573](#2573))
([766a7c1](766a7c1))


### Bug Fixes

* more logging of upload get fail
([#2593](#2593))
([35cc344](35cc344))
* one more typo
([de5582e](de5582e))


### Other Changes

* more deals get debugging
([#2594](#2594))
([36a26f1](36a26f1))
* remove debugging
([#2595](#2595))
([3dc7916](3dc7916))
* remove legacy upload codepath
([#2580](#2580))
([88c05a9](88c05a9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants