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

fix: [Approach 2] Watchdog does not shut down executor on client closing if the executor is provided by ExecutorProvider. #1883

Closed
wants to merge 2 commits into from
Closed

fix: [Approach 2] Watchdog does not shut down executor on client closing if the executor is provided by ExecutorProvider. #1883

wants to merge 2 commits into from

Conversation

blakeli0
Copy link
Contributor

@blakeli0 blakeli0 commented Nov 15, 2022 •
edited

This approach has fewer code changes, but it makes shouldAutoClose not the only source of truth for us to decide should we close the Watchdog(and the executor) or not, it is now implicitly decided by shouldAutoClose and needExecutor in ClientContext. It may cause some confusion if a customer decides to implement their own WatchdogProvider.

@sonarcloud
Copy link

sonarcloud bot commented Nov 15, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

92.9% 92.9% Coverage
0.0% 0.0% Duplication

@blakeli0 blakeli0 mentioned this pull request Nov 15, 2022
Closed
@meltsufin meltsufin mentioned this pull request Nov 17, 2022
Merged
@blakeli0
Copy link
Contributor Author

blakeli0 commented Dec 8, 2022

Closing in favor of #1890 and #1884

@blakeli0 blakeli0 closed this Dec 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@blakeli0