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.

Sign up for GitHub

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

Jump to bottom

Use sys.exit instead of exit #5627

Merged
merged 2 commits into from
Aug 9, 2022
Merged

Use sys.exit instead of exit #5627

merged 2 commits into from
Aug 9, 2022

Conversation

varun-dc
Copy link
Contributor

@varun-dc varun-dc commented Aug 8, 2022 •
edited
Loading

resolves #5621

Description

exit is implicitly imported, but it's not recommended to use it in programs (see linked issue for more details).
Changes the usage to use sys.exit instead.

Checklist

@varun-dc varun-dc requested review from a team and ChenyuLInx August 8, 2022 15:19
@cla-bot cla-bot bot added the cla:yes label Aug 8, 2022
@varun-dc varun-dc requested a review from a team as a code owner August 8, 2022 15:20
@varun-dc varun-dc requested a review from VersusFacit August 8, 2022 15:20
@jtcohen6 jtcohen6 added Team:Execution ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering backport 1.2.latest This PR will be backported to the 1.2.latest branch labels Aug 8, 2022
ChenyuLInx
ChenyuLInx approved these changes Aug 9, 2022
View reviewed changes
Copy link
Contributor

@ChenyuLInx ChenyuLInx left a comment

Choose a reason for hiding this comment

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

LGTM! @varun-dc Thanks for the super fast contribution!

@ChenyuLInx ChenyuLInx merged commit 4d6208b into dbt-labs:main Aug 9, 2022
@jtcohen6 jtcohen6 added backport 1.2.latest This PR will be backported to the 1.2.latest branch and removed backport 1.2.latest This PR will be backported to the 1.2.latest branch labels Aug 10, 2022
SOVALINUX pushed a commit to SOVALINUX/dbt-core that referenced this pull request Aug 12, 2022
@jtcohen6 @varun-dc
Use sys.exit instead of exit (dbt-labs#5627) (dbt-labs#5640)
39901c5 
* Use sys.exit instead of exit

* Add changelog

Co-authored-by: varun-dc <[email protected]>
@varun-dc varun-dc deleted the fix/sys-exit branch August 16, 2022 13:39
VersusFacit pushed a commit that referenced this pull request Sep 5, 2022
@varun-dc @VersusFacit
Use sys.exit instead of exit (#5627)
d6a28cb 
* Use sys.exit instead of exit

* Add changelog
agoblet pushed a commit to BigDataRepublic/dbt-core that referenced this pull request Sep 16, 2022
@varun-dc
Use sys.exit instead of exit (dbt-labs#5627)
028b236 
* Use sys.exit instead of exit

* Add changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChenyuLInx ChenyuLInx ChenyuLInx approved these changes

@VersusFacit VersusFacit Awaiting requested review from VersusFacit VersusFacit was automatically assigned from dbt-labs/core-group

Assignees
No one assigned
Labels
backport 1.2.latest This PR will be backported to the 1.2.latest branch cla:yes ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CT-999] [Bug] In 1.2.0 implictly imported exit being used instead of sys.exit
3 participants
@varun-dc @ChenyuLInx @jtcohen6