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

Fix regression in OpenVPN monitor #5282

Merged
merged 1 commit into from
Oct 12, 2023
Merged

Fix regression in OpenVPN monitor #5282

merged 1 commit into from
Oct 12, 2023

Conversation

dlon
Copy link
Member

@dlon dlon commented Oct 12, 2023

Changes:

  • Fix regression on main -- the openvpn monitor panics. This seems to be a regression due to the recent cleanup.
  • Simplify synchronization significantly.
  • The wait functions are all async now. There are no calls to block_on, etc.
  • The only async-sync boundary is in the parent tunnel monitor (which is fully synchronous), which improves readability.
  • Other improvements.

Close DES-404.


This change is Reviewable

@linear
Copy link

linear bot commented Oct 12, 2023

DES-404 OpenVPN monitor crashes on main

There's currently a panic when setting the tunnel protocol to OpenVPN on main.

@dlon dlon force-pushed the openvpn-mon-async branch 2 times, most recently from b77b02f to cf48e51 Compare October 12, 2023 11:35
Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 3 files at r1, 1 of 2 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@dlon dlon force-pushed the openvpn-mon-async branch from e62f1da to 7f8b7e9 Compare October 12, 2023 13:05
@dlon dlon merged commit 7f8b7e9 into main Oct 12, 2023
28 checks passed
@dlon dlon deleted the openvpn-mon-async branch October 12, 2023 13:06
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.

2 participants