Install deps and project in one go #4
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
May 4, 2022 11:45
This works around a change in poetry 1.2; see explanation here: matrix-org/synapse#12626 (comment) I had originally made the "install deps" and "install project" steps separate, to allow us to skip the installing deps bit if there was a cache hit. Maintaining that approach would now be painful because I'd need two different `poetry install` invocations: one with extras and one without. And it only saved us 1 or 2 seconds per run.
but we want to leave it doing so for Reasons
|
Once merged, I'll make a 1.1.2 release and push the v1 tag forward. Should resolve matrix-org/synapse#12626. |
|
Tested by manually running the latest deps job against matrix-org/synapse@3630220. That run is still in progress, but https://github.com/matrix-org/synapse/actions/runs/2269925714 shows that mypy passed. |
anoadragon453
approved these changes
May 5, 2022
DMRobertson
commented
May 6, 2022
Comment on lines
-82
to
+97
| - name: Install base dependencies only | ||
| if: steps.poetry-venv-cache.outputs.cache-hit != 'true' && inputs.extras == '' | ||
| - name: Install project (no extras) | ||
| if: inputs.extras == '' | ||
| run: poetry install --no-interaction --no-root | ||
| shell: bash | ||
|
|
||
| - name: Install poetry with --extras=${{ inputs.extras }} | ||
| if: steps.poetry-venv-cache.outputs.cache-hit != 'true' && inputs.extras != '' | ||
| run: poetry install --no-interaction --no-root --extras="${{ inputs.extras }}" | ||
| shell: bash | ||
|
|
||
| # (Not sure if this is needed if there's a cache hit?) | ||
| - name: Install project | ||
| run: poetry install --no-interaction | ||
| - name: Install project with --extras=${{ inputs.extras }} | ||
| if: inputs.extras != '' | ||
| run: poetry install --no-interaction --extras="${{ inputs.extras }}" |
There was a problem hiding this comment.
There is a mistake here. Line +92 should not have --no-root. Its presence now means that we don't install the package in question when no extras are to be installed.
Didn't spot this when testing because the latest deps job doesn't install with no-extras.
DMRobertson
pushed a commit
that referenced
this pull request
May 6, 2022
Regression introduced in #4, see #4 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This works around a change in poetry 1.2; see explanation here:
matrix-org/synapse#12626 (comment)
I had originally made the "install deps" and "install project" steps
separate, to allow us to skip the installing deps bit if there was a
cache hit. I I don't mind losing this, for two reasons:
need two different
poetry installinvocations: one with extras and onewithout.