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

refactor: pass context for goroutine shutdown #1187

Merged
merged 4 commits into from Oct 7, 2023
Merged

refactor: pass context for goroutine shutdown #1187

merged 4 commits into from Oct 7, 2023

Conversation

0xERR0R
Copy link
Owner

@0xERR0R 0xERR0R commented Oct 6, 2023

  • pass context to server/resolvers
  • each goroutine checks for cancellation
  • I think, maybe we'll need it for config reload -> we can just close the context and recreate all resolvers

@0xERR0R 0xERR0R added 🔨 enhancement New feature or request 🧰 technical debts Technical debts, refactoring labels Oct 6, 2023
@0xERR0R 0xERR0R added this to the v0.23 milestone Oct 6, 2023
@0xERR0R 0xERR0R enabled auto-merge (squash) October 6, 2023 07:52
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Files Coverage Δ
cache/expirationcache/expiration_cache.go 100.00% <100.00%> (ø)
cache/expirationcache/prefetching_cache.go 100.00% <100.00%> (ø)
cmd/serve.go 95.16% <100.00%> (+0.24%) ⬆️
config/config.go 78.57% <100.00%> (+0.36%) ⬆️
lists/list_cache.go 95.65% <100.00%> (ø)
querylog/database_writer.go 100.00% <100.00%> (ø)
resolver/blocking_resolver.go 96.14% <100.00%> (+0.05%) ⬆️
resolver/bootstrap.go 95.47% <100.00%> (+0.02%) ⬆️
resolver/caching_resolver.go 94.68% <100.00%> (+0.11%) ⬆️
resolver/client_names_resolver.go 100.00% <100.00%> (ø)
... and 4 more

📢 Thoughts on this report? Let us know!.

Copy link
Collaborator

@ThinkChaos ThinkChaos left a comment

Choose a reason for hiding this comment

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

This is great!

cmd/serve.go Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
@kwitsch
Copy link
Collaborator

kwitsch commented Oct 7, 2023

Agreed this looks really great. 👍

0xERR0R and others added 2 commits October 7, 2023 21:12
Co-authored-by: ThinkChaos <ThinkChaos@users.noreply.github.com>
@0xERR0R 0xERR0R merged commit 33ea933 into main Oct 7, 2023
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the context branch October 7, 2023 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 enhancement New feature or request 🧰 technical debts Technical debts, refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants