Add standalone implementation of v1 Relay#1186
Merged
Merged
Conversation
Intended to be used by standalone daemons and tests
Contributor
|
Do we have to do this? Can't we just leave the code in go-libp2p-circuit for the transition period? Then we wouldn't have to review a 1340 LOC PR, but could just use code that we know works in production. |
Contributor
Author
|
The bultin implementation of go-libp2p-circuit is deficient; it cant be
used safely as only a relay and has resource management problems.
This remedies the situation, by extracting and fixing.
I would very much do this and kill the other repo. Originally I was going
to put the code in the relay daemon, but I decided to put it here for
generality and completeness.
Review should be easy, the mods are small and the code is similar to the v2
relay code. There is nothing exotic to consider. The bulk of the code is
protobuf, review is neede for some 200loc.
…On Wed, Sep 15, 2021, 11:28 Marten Seemann ***@***.***> wrote:
Do we have to do this? Can't we just leave the code in go-libp2p-circuit
for the transition period? Then we wouldn't have to review a 1340 LOC PR,
but could just use code that we know works in production.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1186 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAI4SWP7N2YKBTOOQB3SCDUCBKMJANCNFSM5D7XYVRQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
marten-seemann
approved these changes
Sep 17, 2021
Contributor
marten-seemann
left a comment
There was a problem hiding this comment.
I haven't done an in-depth review, since this is mostly code from go-libp2p-circuit.
Why is go-libp2p-circuit not removed from go.mod in this PR?
Contributor
Author
|
still have some open todos, see the pr description. I plan to finish it
today.
…On Fri, Sep 17, 2021, 11:33 Marten Seemann ***@***.***> wrote:
***@***.**** approved this pull request.
I haven't done an in-depth review, since this is mostly code from
go-libp2p-circuit.
Why is go-libp2p-circuit not removed from go.mod in this PR?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1186 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAI4SSZWPFKOM7MUTVRR4LUCL4PLANCNFSM5D7XYVRQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Contributor
Author
|
Turns out we need to keep a reference in go.mod for now, we have a compatiblity test which tests whether the v2 client can connect to legacy clients. |
a14953a to
8e5996b
Compare
8e5996b to
243d07b
Compare
Contributor
Author
|
marked as ready -- @marten-seemann care for another pass before we merge? |
marten-seemann
approved these changes
Sep 20, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds an implementation of a v1 Relay, extracted and slightly modified for a bit of resource management, from go-libp2p-circuit, intended to be used by standalone daemon implementations and tests.
This allows us to deprecate and completely dereference the go-libp2p-circuit repo.
TBD: