feat: Use Setpgid=true on Unix systems so that signals are not sent to the child process #664
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
disq
approved these changes
Feb 7, 2023
Member
Author
|
Oh okay, this field is not even available on Windows :) I will refactor a bit |
Codecov ReportBase: 47.16% // Head: 47.21% // Increases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #664 +/- ##
==========================================
+ Coverage 47.16% 47.21% +0.04%
==========================================
Files 70 70
Lines 6736 6742 +6
==========================================
+ Hits 3177 3183 +6
Misses 3111 3111
Partials 448 448
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
⏱️ Benchmark results
|
yevgenypats
approved these changes
Feb 8, 2023
kodiakhq bot
pushed a commit
that referenced
this pull request
Feb 8, 2023
🤖 I have created a release *beep* *boop* --- ## [1.36.0](v1.35.0...v1.36.0) (2023-02-08) ### Features * Add table diff methods ([#668](#668)) ([f6baa82](f6baa82)) * Use Setpgid=true on Unix systems so that signals are not sent to the child process ([#664](#664)) ([2883487](2883487)) ### Bug Fixes * Remove duplicate force implementation ([#670](#670)) ([fe34554](fe34554)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
kodiakhq bot
pushed a commit
to cloudquery/cloudquery
that referenced
this pull request
Feb 27, 2023
…ruptly (#7756) Before, when pressing ctrl+c, these messages would show in the CLI: ``` failed to terminate destination client: destination plugin process exited with status signal: killed failed to terminate source client: source plugin process exited with status signal: killed ``` This PR fixes it so that these messages no longer appear. This is the main user-facing change. Other changes include: - sending an analytics message when the sync ends for any reason, rather than only on successful syncs - include an "exit reason" in the analytics message, so that we can better understand when and why syncs fail - introduction of an `CQ_ANALYTICS_HOST` environment variable that can override the default host. This is mainly useful for testing, so no code changes are necessary to send analytics to a test server. For normal operation, users should still use `--telemetry-level` or `CQ_TELEMETRY_LEVEL` to control the telemetry - send analytics not only for the GitHub registry, but also for path and grpc registries. This can help us better understand how plugins are being deployed. We strip off any potentially identifying information and send only the final part of the path (i.e. the name of the binary) This change will be improved by cloudquery/plugin-sdk#664 but doesn't depend on it to work.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By default, commands in Go are started in the same process group. When a signal is sent (e.g. ctrl+c), this gets sent to the entire group, causing plugins to shut down before the CLI has a chance to do closing operations and then relay the signal. By setting
Setpgid: trueon UNIX systems, we can get better control over the shutdown process of child processes.