Use hash of all requirements files in cache keys for ci #6061
Conversation
| @@ -280,7 +280,7 @@ jobs: | |||
| - uses: actions/cache@v2 | |||
| with: | |||
| path: ${{ env.pythonLocation }} | |||
| key: ${{ env.pythonLocation }}-${{ hashFiles('setup.py') }}-${{ hashFiles('dev-requirements.txt') }} | |||
| key: ${{ env.pythonLocation }}-${{ hashFiles('**/requirements.txt', 'dev_tools/requirements/**/*.txt') }} | |||
There was a problem hiding this comment.
Previously, the key was sensitive to setup.py, too. Should we keep that? It seems to me safer to do so.
Also, if you agree we should keep sensitivity to setup.py, then it might be prudent to broaden it to all setup.py files.
There was a problem hiding this comment.
I don't think that's necessary since we don't install any of the cirq modules themselves in ci, we just run tests from the checked out cirq source. This may have come from very early days when we declared some dependencies in setup.py itself, but we stopped doing that probably even before we got rid of dev-requirements.txt.
There was a problem hiding this comment.
IIUC, the cached content is for the Python environment content after pip install -r some/requirement.txt
We don't install cirq as check/pytest works directly with repository sources.
If so, we should not hash setup.py as it is not used in the CI run.
I ran into some strange CI failures while trying to update mypy in #6059 and tracked them down to a bad cache of python dependencies. While looking at this I realized that the way we compute a cache key for python dependencies in CI is broken, since it doesn't actually take all requirements files into account.