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

ping/: Update to rust-libp2p release v0.48.0 and master v0.49.0 #41

Merged
merged 2 commits into from
Sep 25, 2022

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Sep 10, 2022

See release of v0.48.0 in libp2p/rust-libp2p#2869.

Master bump to v0.49.0 in libp2p/rust-libp2p#2813

Followed the instructions on https://github.com/libp2p/test-plans#how-to-add-a-new-version-to-pingrust.

I skipped step (3), namely to run the new test on my local machine. As far as I can tell this is done on CI here already. Please correct me if I am wrong.

@BigLep
Copy link
Contributor

BigLep commented Sep 12, 2022

@mxinden : do we have a corresponding step in the rust-libp2p release process to remember to do this or to verify this was done?

@mxinden
Copy link
Member Author

mxinden commented Sep 15, 2022

@BigLep at this point we don't and I would argue that in itself is not a big issue. The only thing that is happening after a release is that the rust-libp2p CI fails. It does not affect the go-libp2p CI.

I have to give this more thought. Ideally this step would not be required in the first place. See also libp2p/rust-libp2p#2813 (comment)

@mxinden
Copy link
Member Author

mxinden commented Sep 15, 2022

@laurentsenta can you take a look at the test failure? I am not sure what is the reason for it. In case you know it already, great. If not, I can take a more in-depth look.

@BigLep
Copy link
Contributor

BigLep commented Sep 16, 2022

@mxinden : I don't know all that involved here but the reasons I chimed in are:

  1. I was under the impression that we aren't running the fully compatibility test-plan as part of each PR but rather a lighter version. As a result, I assume before we do a release we ensure the full compatibility test-plan passes.
  2. I think we want it so that any maintainer could be shuffled in an know how to execute a release. I assume there are are a set of manual steps including updating version numbers some places (great if that's automated though), making public announcements, etc. That should be standardized so anyone can know how to do it and not miss anything.

@mxinden
Copy link
Member Author

mxinden commented Sep 16, 2022

I was under the impression that we aren't running the fully compatibility test-plan as part of each PR but rather a lighter version. As a result, I assume before we do a release we ensure the full compatibility test-plan passes.

If I am not mistaken, we are running the full test suit on each rust-libp2p pull request. (Which I am in favor of.)

I think we want it so that any maintainer could be shuffled in an know how to execute a release. I assume there are are a set of manual steps including updating version numbers some places (great if that's automated though), making public announcements, etc. That should be standardized so anyone can know how to do it and not miss anything.

👍 agree with this long-term. On documenting this specific release step, i.e. bumping the versions in libp2p/test-plans, I suggest we think about whether we can get rid of it first. All of this is relatively new, thus there might be room for improvement.

On the general matter, i.e. enabling folks other than Max to make rust-libp2p releases, this is related to libp2p/rust-libp2p#2902 and needs some focus. Though I am not sure how to prioritize it with libp2p Day and the WebRTC efforts. Thoughts?

@mxinden
Copy link
Member Author

mxinden commented Sep 21, 2022

Friendly ping @laurentsenta. Any thoughts on the CI failures?

@elenaf9 let me know in case you found a solution in #42 and I am missing something.

@laurentsenta
Copy link
Collaborator

Thanks for the bump @mxinden on it,

The plan:

  1. Fix the master build error to get rid of the current CI-wide error (rust-libp2p build breaks on master's update #43)
  2. Then merge the write-artifact patch
  3. Which mean we can merge this PR (ping/: Update to rust-libp2p release v0.48.0 and master v0.49.0 #41)
  4. Which mean we can merge ping/rust: adapt to rust-sdk patch, remove unused dependencies #42

@laurentsenta
Copy link
Collaborator

On the larger discussion regarding what we tests, releases process, etc:

@BigLep is right with:

I was under the impression that we aren't running the fully compatibility test-plan as part of each PR but rather a lighter version. As a result, I assume before we do a release we ensure the full compatibility test-plan passes.

With libp2p/rust-libp2p#2835, we enabled the cross version test and the "small" cross-implementation test. The "small" tests uses latest versions only (and takes ~10 minutes).

The big test takes >30 minutes, my plan is to propose this as a step in (go|rust)-libp2p release processes.

You two have different info, sorry about that: we talked about it during a 1:1 with Steve, then I went on break. I shared a few notes below, but I haven't had time to properly implement and communicate these with you @mxinden.

@laurentsenta
Copy link
Collaborator

laurentsenta commented Sep 23, 2022

@mxinden, I took the liberty to rebase over the latest master, please feel free to review and merge. We might have caught a regression:

In https://github.com/libp2p/test-plans/actions/runs/3111852289/jobs/5044617290 > "Run the composition file" > search "container exited" (around line 475), we get various errors then the go side exists, and the rust side more or less hangs.

@mxinden
Copy link
Member Author

mxinden commented Sep 25, 2022

Regression might be due to libp2p/rust-libp2p#2860. I am investigating @laurentsenta.

@mxinden
Copy link
Member Author

mxinden commented Sep 25, 2022

please feel free to review and merge

I will go ahead here. Thanks @laurentsenta for the help!

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