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

feat: api support for ipfs-cluster@1.0 #1789

Merged
merged 3 commits into from
Apr 12, 2022
Merged

feat: api support for ipfs-cluster@1.0 #1789

merged 3 commits into from
Apr 12, 2022

Conversation

olizilla
Copy link
Contributor

@olizilla olizilla commented Apr 8, 2022

The IPFS Cluster v1 release has much improved perf, and some breaking API changes.

In this PR we upgrade the ipfs-cluster client and local test environment to work with the v1 cluster API.

CI was failing for the API, so this PR includes a change to that workflow to fix that.

After merging this PR

  • Update staging env cloudflare worker secrets for CLUSTER_SERVICE to IpfsCluster & CLUSTER_BASIC_AUTH_TOKEN from 1pass - to use the new staging cluster running ipfs-cluster@1.0.0-rc.4
  • open a follow PR to update the cron package dep on ipfs-cluster client.

see: #1736
see: https://github.com/nftstorage/ipfs-cluster/releases/tag/v5.0.0

License: (Apache-2.0 AND MIT)
Signed-off-by: Oli Evans oli@tableflip.io

The IPFS Cluster v1 release has much improved perf, and some breaking API changes.

In this PR we upgrade the ipfs-cluster client and local test environment to work with the v1 cluster API.

see: #1736
see: https://github.com/nftstorage/ipfs-cluster/releases/tag/v5.0.0

License: (Apache-2.0 AND MIT)
Signed-off-by: Oli Evans <oli@tableflip.io>
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 8, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: bb7e144
Status: ✅  Deploy successful!
Preview URL: https://2b0a615e.nft-storage-1at.pages.dev

View logs

@olizilla
Copy link
Contributor Author

olizilla commented Apr 8, 2022

@gobengo CI is failing on this PR with

Invalid workflow file: .github/workflows/api.yml#L31
The workflow is not valid. .github/workflows/api.yml (Line: 31, Col: 9): Unrecognized named-value: 'secrets'. Located at position 75 within expression: github.event_name == 'pull_request' && github.ref != 'refs/heads/main' && secrets.CF_API_TOKEN != null

It looks like this was introduced with #1698

the secrets context is not available in `jobs.<job id>.if`
see: https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability

License: (Apache-2.0 AND MIT)
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla olizilla requested review from gobengo and vasco-santos April 8, 2022 15:21
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.

Cluster related changes look good. secrets change still doesn't make as it looks like dev deploy fails. I will check and see if understand why this if is in use here

@gobengo
Copy link
Contributor

gobengo commented Apr 8, 2022

@olizilla big apologies! I should have tested all the workflows since I touched them. Is there a better way than briefly updating some file in each project?

Here is another approach that might work? #1791

@vasco-santos if you're still looking, wdyt of that?

@gobengo
Copy link
Contributor

gobengo commented Apr 8, 2022

I will check and see if understand why this if is in use here

I added the conditional to make PRs pass this check even if coming from forks that don't have that variable defined. I had a PR from my fork that was failing due to it.

I think something else is going on where the existing secret.CF_API_TOKEN is defined in this repo but erroring against CF API. i.e. it's potentially invalid (or CF API is rejecting it, but that seems less likely).

@gobengo
Copy link
Contributor

gobengo commented Apr 8, 2022

@olizilla I think merging That PR, which is into the source branch of this pr, might make it pass. (unless secret.CF_API_TOKEN is invalid and CF rejects it)

* remove secrets check in if: in github api workflow

* touch api package to trigger github action

* ci(api): Try another way of deploy-dev not breaking if CF_API_TOKEN is not set

* ci(api): temporarily make deploy-dev not need test

* Revert "touch api package to trigger github action"

This reverts commit 1f110d1.

* Revert "ci(api): temporarily make deploy-dev not need test"

This reverts commit d09c94f.

* feat: api support for ipfs-cluster@1.0

The IPFS Cluster v1 release has much improved perf, and some breaking API changes.

In this PR we upgrade the ipfs-cluster client and local test environment to work with the v1 cluster API.

see: #1736
see: https://github.com/nftstorage/ipfs-cluster/releases/tag/v5.0.0

License: (Apache-2.0 AND MIT)
Signed-off-by: Oli Evans <oli@tableflip.io>

Co-authored-by: Oli Evans <oli@tableflip.io>
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

@vasco-santos vasco-santos merged commit a9d6f84 into main Apr 12, 2022
@vasco-santos vasco-santos deleted the cluster-v1 branch April 12, 2022 08:05
alanshaw pushed a commit that referenced this pull request Apr 12, 2022
alanshaw pushed a commit that referenced this pull request Apr 12, 2022
Follow up of #1789 for today's migration. Should only be merged after api release #1749
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.

3 participants