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

feature: CancellationToken-based shutdowns #5325

Merged
merged 9 commits into from
Jan 13, 2025

Conversation

jstuczyn
Copy link
Contributor

@jstuczyn jstuczyn commented Jan 9, 2025

this PR introduces scaffolding for using CancellationToken and TaskTracker for our graceful shutdowns rather than the existing TaskClient and TaskManager.

it focues on creating the new API and migrating some of the existing tasks inside NymNode


This change is Reviewable

@jstuczyn jstuczyn added this to the Hu milestone Jan 9, 2025
@jstuczyn jstuczyn requested a review from octol January 9, 2025 15:00
Copy link
Contributor

@octol octol left a comment

Choose a reason for hiding this comment

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

I wonder what the impact is when now switching to not-dropguard behaviour by default? I know in the vpn client we seem to hit the drop guards for the gateway-client

common/task/src/cancellation.rs Outdated Show resolved Hide resolved
common/task/src/cancellation.rs Show resolved Hide resolved
common/task/src/cancellation.rs Outdated Show resolved Hide resolved
@@ -447,6 +448,10 @@ impl NymNode {
wireguard: Some(wireguard_data),
config,
accepted_operator_terms_and_conditions: false,
shutdown_manager: ShutdownManager::new("NymNode")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why this one PascalCase while the others are kebab-case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I've been particularly consistent, but I think with recent changes I attempted to use PascalCase for the top level name and kebab-case or snake_case for any child components

Copy link

vercel bot commented Jan 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
docs-nextra ⬜️ Ignored (Inspect) Visit Preview Jan 10, 2025 10:44am
nym-next-explorer ⬜️ Ignored (Inspect) Visit Preview Jan 10, 2025 10:44am

@jstuczyn
Copy link
Contributor Author

jstuczyn commented Jan 10, 2025

I wonder what the impact is when now switching to not-dropguard behaviour by default? I know in the vpn client we seem to hit the drop guards for the gateway-client

i think it would have to be dealt with on case-by-case basis, because with those changes on-drop behaviour is explicit

@jstuczyn jstuczyn merged commit 102cd10 into develop Jan 13, 2025
20 checks passed
@jstuczyn jstuczyn deleted the feature/cancellation-token-shutdowns branch January 13, 2025 09:13
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