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

Ignore sigterm when closing source/destination #20519

Merged
merged 11 commits into from
Jan 3, 2023

Conversation

jdpgrailsdev
Copy link
Contributor

What

  • Reduce error noise when closing sources/destinations

How

  • Ignore the SIGTERM exit code (143) in addition to exit code 0 when closing source/destination processes.

N.B.: I decided to put the set of ignored exit codes in both classes, as over time we may end up with a different set that we want to ignore for one but not both. I don't hold this belief strongly, so if we'd rather extract it to a common utility to avoid duplication, let me know.

Recommended reading order

  1. DefaultAirbyteSource.java
  2. DefaultAirbyteDestination.java

Tests

  • All tests pass
  • Kube acceptance tests pass locally
  • Project builds successfully locally

Copy link
Contributor

@davinchia davinchia left a comment

Choose a reason for hiding this comment

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

Great idea!

Can we pull the exit codes out into a separate class so they don't diverge?

Feel free to merge after.

Note: Worth throwing up a quick test to make sure we don't remove this functionality?

@jdpgrailsdev
Copy link
Contributor Author

@davinchia I debated pulling the codes into a common place. On one hand, reducing repeated code is always a good thing. On the other hand, we may end up with different codes that we allow for a source vs destination. If we don't think that is a concern, then I am happy to move it to shared place.

I will see about adding a test to cover the additional exit code.

@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets December 21, 2022 18:51 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets December 21, 2022 18:52 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 14:10 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 14:11 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 14:27 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 14:27 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 15:28 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 15:29 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 16:34 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 16:35 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 17:20 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 17:20 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 17:24 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 17:24 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 17:36 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 17:36 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 18:13 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 18:13 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 18:57 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets January 3, 2023 18:58 — with GitHub Actions Inactive
@jdpgrailsdev jdpgrailsdev merged commit a5fcbaa into master Jan 3, 2023
@jdpgrailsdev jdpgrailsdev deleted the jonathan/ignore-sigterm branch January 3, 2023 19:32
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