Check pip-compile failed to resolve requirements#482
Check pip-compile failed to resolve requirements#482thundergolfer merged 6 commits intobazel-contrib:masterfrom
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
@googlebot I signed it! |
thundergolfer
left a comment
There was a problem hiding this comment.
Suggested adding extra error checking, but happy with the contributed fix 🙏
| sys.exit(1) | ||
| except SystemExit: | ||
| except SystemExit as e: | ||
| if e.code == 2: |
There was a problem hiding this comment.
This improves things for sure, but still leaves us open to cases where e.code not in {0, 2}. Looking at the piptools source, it only ever exits 0 or 2, but my preference would be to check for the unexpected cases and exit(1) instead of incorrectly succeeding .
if e.code == 2:
# handle 2
# ...
sys.exit(1)
elif e.code == 0:
# handle 0
# ...
sys.exit(0)
else:
# handle unexpected
# ...
sys.exit(1)There was a problem hiding this comment.
I totally agree. I have fixed it that way.
|
Thanks for the contribution 👍 |
PR Checklist
Please check if your PR fulfills the following requirements:
.parfiles. See CONTRIBUTING.md for infoPR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #478
When pip-compile fails to resolve the dependency,
requirements_testwill pass regardless of the content of the lock file.What is the new behavior?
When pip-compile fails to resolve the dependency, exit with an error message, and the test will fail.
Does this PR introduce a breaking change?
Other information