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

Use a unique exit code to identify baseline changes #214

Merged
merged 2 commits into from
Aug 4, 2019

Conversation

lirantal
Copy link
Contributor

@lirantal lirantal commented Jul 31, 2019

Addresses the issue raised in #212 to exit with a unique exit code number so that this can be identified by scripts and programs wrapping and invoking the detect-secrets-hook to run specific flows.

Note: I intentionally didn't use exit code 2 since that is part of standardised error numbers such as those used for shell related errors so to avoid a conflict.

Copy link
Contributor

@domanchi domanchi left a comment

Choose a reason for hiding this comment

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

Looks sane to me after tests pass.

@lirantal
Copy link
Contributor Author

lirantal commented Aug 3, 2019

@domanchi could you check about the failing build? doesn't seem related to this change

@KevinHock
Copy link
Collaborator

I’m on my phone so it’s a little harder to see but I think it’s just another newline needed before and after the new function in the pre commit hook test (lines 21 and 24)

@lirantal
Copy link
Contributor Author

lirantal commented Aug 3, 2019

uhmm, there is a new line though... ?

image

@domanchi
Copy link
Contributor

domanchi commented Aug 4, 2019

https://travis-ci.org/Yelp/detect-secrets/jobs/567252558#L569

Travis runs our pre-commit hook as a style linter, and modifies files accordingly. Looks like you need two new lines between a root defined function.

Best to install the pre-commit framework, and let that fix files for you.

$ tox -e pre-commit -- run 

@KevinHock KevinHock merged commit 2630cb5 into Yelp:master Aug 4, 2019
@KevinHock
Copy link
Collaborator

KevinHock commented Aug 4, 2019

No worries, 👍

I'll add pep8 newlines, now that I'm on Desktop

Thanks again for making this :D

@lirantal
Copy link
Contributor Author

lirantal commented Aug 4, 2019

Thanks fellas for all the help! 👌

killuazhu pushed a commit to IBM/detect-secrets that referenced this pull request May 28, 2020
* Timeout DB2 detector if it takes too long

* Trying to fix tests

* Use multiprocessing instead of signal

* Faster tests

* whitespace and comments

* Print 'DB2Detector timed out.'

* Use SQL_ATTR_LOGIN_TIMEOUT

* Fix tests

* ConnectTimeout

* Error message

* Addressed @xianjun comments

* Timeout is UNVERIFIED, other exceptions are VERIFIED_FALSE

* Addressed @xianjun comment
killuazhu pushed a commit to IBM/detect-secrets that referenced this pull request Jul 9, 2020
* Timeout DB2 detector if it takes too long

* Trying to fix tests

* Use multiprocessing instead of signal

* Faster tests

* whitespace and comments

* Print 'DB2Detector timed out.'

* Use SQL_ATTR_LOGIN_TIMEOUT

* Fix tests

* ConnectTimeout

* Error message

* Addressed @xianjun comments

* Timeout is UNVERIFIED, other exceptions are VERIFIED_FALSE

* Addressed @xianjun comment
killuazhu pushed a commit to IBM/detect-secrets that referenced this pull request Sep 17, 2020
Supports git-defenders/detect-secrets-discuss#190

DB2 Verification (Yelp#196)

Supports git-defenders/detect-secrets-discuss#190

Use DB2 detector (Yelp#199)

Supports git-defenders/detect-secrets-discuss#190

Refactor DB2 verification for calling externally (Yelp#203)

Supports fixing bug [here](https://github.ibm.com/git-defenders/detect-secrets-stream/blob/master/detect_secrets_stream/validation/db2.py#L25)

Catch DB2 hostname, port, database from connection url (Yelp#209)

Supports git-defenders/detect-secrets-discuss#212

Timeout DB2 detector if it takes too long (Yelp#214)
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.

None yet

3 participants