search

lervag / apy

CLI script for interacting with local Anki collection
MIT License
243 stars 17 forks source link

GitHub actions issue with caching? #61

Closed lervag closed 1 year ago

lervag commented 1 year ago

After #59 was merged the GitHub actions step fails due to missing dependency of mypy:

image

I believe this is because of this part:

https://github.com/lervag/apy/blob/2aae67373f43d50fd945829d5cd9124b6147561f/.github/workflows/ci.yaml#L21-L29

But I'm not sure exactly what's wrong. I hope @ckp95 or @denismaciel would be able to help me here?

lervag commented 1 year ago

I believe there is a problem with the hash key. From the CICD log, we see that the key is venv-Linux-, i.e. the ${{ hashFiles('**/poetry.lock') }} seems to return an empty string. I assume this is the part that should account for changes to the dependencies in pyproject.toml? Things work again now after I change the key, but I assume that it did not really fix the problem.

ckp95 commented 1 year ago

https://github.com/actions/setup-python#caching-packages-dependencies

The action defaults to searching for a dependency file (requirements.txt or pyproject.toml for pip, Pipfile.lock for pipenv or poetry.lock for poetry) in the repository, and uses its hash as a part of the cache key. Input cache-dependency-path is used for cases when multiple dependency files are used, they are located in different subdirectories or different files for the hash that want to be used.

we don't commit the poetry.lock file so that might be it. we can either change the CI script to check for a changed pyproject.toml, or commit the lockfile.

I didn't commit the lockfile at first because I thought it wouldn't matter, but I have no strong opinion about it.

denismaciel commented 1 year ago

Yeah, I think @ckp95 is right.

From poetry docs... I think apy falls in the application bucket, so I'd vote for committing the lock file.

lervag commented 1 year ago

we don't commit the poetry.lock file so that might be it. we can either change the CI script to check for a changed pyproject.toml, or commit the lockfile.

Yes, that sounds correct. I'm pushing the lockfile now, I think it makes sense for applications to have the lockfile as part of the repo.

lervag commented 1 year ago

The relevant pipeline after adding the lock file: https://github.com/lervag/apy/actions/runs/5197352890.

lervag commented 1 year ago

I believe it is safe to close this issue.