-
Notifications
You must be signed in to change notification settings - Fork 179
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
Run pipenv verify
before sync
#2409
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with uncertainty
@@ -40,7 +40,7 @@ jobs: | |||
- name: Install pipenv | |||
run: pip install pipenv==2023.12.1 | |||
- name: Set up pipenv | |||
run: pipenv sync --dev | |||
run: pipenv verify && pipenv sync --dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure if the &&
here will result in pipenv sync
silently not being run if the verify fails?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure, but I assumed if pipenv verify
fails here the whole workflow will fail, similar to the Makefile.
@@ -16,7 +16,7 @@ ifeq "$(USE_POETRY)" "true" | |||
install-cmd := poetry install | |||
run-cmd := poetry run | |||
else | |||
install-cmd := pipenv sync | |||
install-cmd := pipenv verify && pipenv sync |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure if the &&
here will result in pipenv sync
silently not being run if the verify fails?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The make command prints and errors if the verify fails:
...
Pipfile.lock is out-of-date. Run $ pipenv lock to update.
make: *** [Makefile:64: run-api-server] Error 1
Which seems reasonable to me
Pipenv doesn't automatically check if the
Pipfile
matches thePipfile.lock
when runningpipenv sync
. Addedpipenv verify
commands in all the places to do the manifest/lockfile verification. This should prevent things like #2379 from passing our tests.Poetry seems to automatically do this validation when you run
poetry install
(But there is apoetry check
command as well?)