Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

Tag the hop relay when creating stop streams #77

Merged
merged 3 commits into from
May 22, 2019

Conversation

vyzo
Copy link
Contributor

@vyzo vyzo commented May 22, 2019

This tags the underlying hop relay with the connection manager when we open a new stop stream.
This should avoid the comical situation of dropping the hop relay connection and killing all the relayed connections with it in the connection manager.
On top of #76

@vyzo vyzo requested review from Stebalien and raulk May 22, 2019 13:51
@vyzo vyzo changed the title Feat/stop tags Tag the hop relay when creating stop streams May 22, 2019
raulk
raulk previously requested changes May 22, 2019
Copy link
Member

@raulk raulk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial feedback; still reviewing.

conn.go Outdated Show resolved Hide resolved
conn.go Show resolved Hide resolved
@vyzo vyzo dismissed raulk’s stale review May 22, 2019 15:11

wrong pr; please review only the tag commit.

@raulk raulk changed the base branch from master to fix/stream-close May 22, 2019 16:09
@raulk
Copy link
Member

raulk commented May 22, 2019

@vyzo I changed the target branch of this PR to the correct one.

@vyzo
Copy link
Contributor Author

vyzo commented May 22, 2019

I was planning to merge it on master separately because it solves a different issue, we'll have to change the target branch back again. We can also do this after we merge #76, as it makes it easier to review.

Copy link
Member

@raulk raulk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tagging logic needs docs.

relay.go Show resolved Hide resolved
relay.go Show resolved Hide resolved
conn.go Outdated Show resolved Hide resolved
conn.go Outdated Show resolved Hide resolved
@vyzo vyzo requested a review from raulk May 22, 2019 16:58
Copy link
Member

@raulk raulk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Stebalien Stebalien merged commit 776794b into fix/stream-close May 22, 2019
@Stebalien Stebalien deleted the feat/stop-tags branch May 22, 2019 17:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants