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

updates for libp2p v0.22 #392

Merged
merged 13 commits into from
Oct 6, 2022
Merged

updates for libp2p v0.22 #392

merged 13 commits into from
Oct 6, 2022

Conversation

willscott
Copy link
Collaborator

@willscott willscott commented Sep 19, 2022

should use versioned deps after

ipfs/go-peertaskqueue#25
ipfs/go-ipfs-routing#36

  • needs update of CI test versions to not be against go v1.16 anymore

@welcome
Copy link

welcome bot commented Sep 19, 2022

Thank you for submitting this PR!
A maintainer will be here shortly to review it.
We are super grateful, but we are also overloaded! Help us by making sure that:

  • The context for this PR is clear, with relevant discussion, decisions
    and stakeholders linked/mentioned.

  • Your contribution itself is clear (code comments, self-review for the
    rest) and in its best form. Follow the code contribution
    guidelines

    if they apply.

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
Next steps:

  • A maintainer will triage and assign priority to this PR, commenting on
    any missing things and potentially assigning a reviewer for high
    priority items.

  • The PR gets reviews, discussed and approvals as needed.

  • The PR is merged by maintainers when it has been approved and comments addressed.

We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution.
We are very grateful for your contribution!

@willscott
Copy link
Collaborator Author

is this a new test becoming flaky with these dependency updates?

=== RUN   TestCancellationQueryInProgress
      responsemanager_test.go:161: 
          	Error Trace:	/home/runner/work/go-graphsync/go-graphsync/responsemanager/responsemanager_test.go:161
          	Error:      	[]string{"processRequests(0)", "processRequests(1)"} does not contain "response(0)->abortRequest(0)"
          	Test:       	TestCancellationQueryInProgress
  --- FAIL: TestCancellationQueryInProgress (0.02s)

@willscott
Copy link
Collaborator Author

another flaky test:

=== RUN   TestGraphsyncRoundTripIgnoreNBlocks
      testchain.go:204: 
          	Error Trace:	/home/runner/work/go-graphsync/go-graphsync/impl/testutil.go:133
          	            				/home/runner/work/go-graphsync/go-graphsync/impl/testchain.go:204
          	            				/home/runner/work/go-graphsync/go-graphsync/impl/testchain.go:199
          	            				/home/runner/work/go-graphsync/go-graphsync/impl/graphsync_test.go:568
          	Error:      	response channel never closed
          	Test:       	TestGraphsyncRoundTripIgnoreNBlocks
  --- FAIL: TestGraphsyncRoundTripIgnoreNBlocks (1.03s)

@willscott
Copy link
Collaborator Author

Additional note: required github checks will need to be updated with this change to point to the newer golang versions

@rvagg
Copy link
Member

rvagg commented Sep 20, 2022

TestGraphsyncRoundTripIgnoreNBlocks flaky documented @ #379 and was blocking the unified-ci upgrade. I was holding off #378 because I didn't want such a badly flaky test to be introduced as we've already got a few to deal with here and this one seems particularly flaky.

TestCancellationQueryInProgress I don't recall being flaky, but since it's failing on the tracing then I'm inclined to believe it was—there's a few tests where the tracing is a bit too strict and they need to be wound back a bit.

@rvagg
Copy link
Member

rvagg commented Sep 20, 2022

Other than TestGraphsyncRoundTripIgnoreNBlocks this lgtm, I'm just not super keen on pushing ahead without figuring out why that one's so bad, or coming up with a strategy to stop it being such a problem for CI going forward. I think it's only for x86 so if there's a way to skip it for that then that's probably fine.

Copy link
Collaborator

@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.

LGTM

@hannahhoward hannahhoward merged commit b4ac1a6 into main Oct 6, 2022
dirkmc pushed a commit that referenced this pull request Mar 22, 2023
* bump go.mod to Go 1.18 and run go fix

* stop using the deprecated io/ioutil package

* bump go.mod to Go 1.18 and run go fix

* update .github/workflows/automerge.yml

* update .github/workflows/go-test.yml

* update .github/workflows/go-check.yml

* mechanical updates for libp2p v0.22

* bump testplan to working versions

* bump more versions

* more deps

* fix static checks

* chore(deps): update to tagged deps

Co-authored-by: web3-bot <web3-bot@users.noreply.github.com>
Co-authored-by: willscott <will.scott@protocol.ai>
Co-authored-by: hannahhoward <hannah@hannahhoward.net>
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