Skip to content
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

Poetry cache does not work well with dependency groups #505

Closed
2 of 5 tasks
fredrikaverpil opened this issue Sep 21, 2022 · 4 comments
Closed
2 of 5 tasks

Poetry cache does not work well with dependency groups #505

fredrikaverpil opened this issue Sep 21, 2022 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@fredrikaverpil
Copy link

fredrikaverpil commented Sep 21, 2022

Description:

When Poetry 1.2 dependency groups are defined in pyproject.toml and actions/setup-python@v4 caching is enabled with cache: "poetry", the caching does not work very well.

Let's say you have a few dependency groups which you install in your CI with different kind of commands:

Dependency group Install command
main poetry install --only main
docs poetry install --only docs
linting poetry install --only linting
testing poetry install --only main,testing
typing poetry install --only main,testing,typing

Defined like this (semi-real world example) in pyproject.toml:

[tool.poetry.dependencies]
python = ">=3.10, <3.11"
fastapi = "^0.79.0"
uvicorn = {extras = ["standard"], version = "^0.18.2"}
python-dateutil = "^2.8.2"
pytz = ">2018.5"
redis = "^4.0.2"

[tool.poetry.group.docs.dependencies]
mkdocs = "^1.3.1"
mkdocs-awesome-pages-plugin = "^2.7.0"
mkdocs-material = "^8.2.11"

[tool.poetry.group.linting.dependencies]
black = "^22.1.0"
flake8 = "^4.0.1"
flake8-bandit = "^3.0.0"
flake8-bugbear = "^22.1.11"
isort = "^5.10.1"
pre-commit = "^2.15.0"

[tool.poetry.group.testing.dependencies]
coverage = "^6.2"
factory-boy = "2.12.0"
freezegun = "~=1.0"
pytest = "^7.1.1"
pytest-cov = "^3.0.0"
pytest-dotenv = "^0.5.2"
pytest-freezegun = "^0.4.2"
pytest-mock = "^3.7.0"
pytest-randomly = "^3.11.0"
responses = "0.20.0"

[tool.poetry.group.typing.dependencies]
mypy = "0.*"
types-freezegun = "^1.1.3"
types-protobuf = "^3.19.21"
types-python-dateutil = "^2.8.2"
types-pytz = "^2022.2.1"
types-redis = "^4.3.19"
types-requests = "^2.26.0"

Let's say you then use the following for all these CI jobs:

    - uses: actions/setup-python@v4
      with:
        python-version: ${{ inputs.python-version }}
        cache: 'poetry'

    - run: <install-command>

Whichever cache is written first will be used for all these CI jobs. Let's say the docs dependency group finishes first and will be written as the cache.

The end result seems to be here that all the other dependency groups will only get the docs deps cached. And in this example they are not needed for any of the other dependency groups. So for example, the typing CI job will always have to install all its dependencies even if a cache exists.

Now, I'm wondering if I can somehow utilize the cache-dependency-path option to e.g. set this for each CI job so to produce one cache per dependency group. Or is this use case simply not supported today?

It's possible that this won't be supported and if so, I think the docs should include a recommendation on e.g. not using the built-in caching and instead rely on something like this:

env:
    python-version: ???
    dependency-group: ???

...

    - uses: actions/cache@v3
      with:
        path: |
          ~/.cache/pip
          ~/.cache/pypoetry/virtualenvs
          ~/.cache/pypoetry/cache
          ~/.cache/pypoetry/artifacts
        key: ${{ runner.os }}-${{ runner.arch }}-python${{ env.python-version }}-${{ hashFiles('poetry.lock') }}-depgroup${{ env.dependency-group }}

Action version:
Specify the action version

Platform:

  • Ubuntu
  • macOS
  • Windows

Runner type:

  • Hosted
  • Self-hosted

Tools version:

Repro steps:

  1. Add a dependency group no#1 with just one dependency, which will be the one to generate the cache.
  2. Add another dependency group no#2 which will use a lot of dependencies.
  3. Set up one GHA CI job for each group using actions/setup-python@v4 and specify cache: 'poetry'.
  4. See how the CI job using the dependency group no#2 will never get its dependencies cached and always keep installing them.

Expected behavior:

I'd expect to be able to append e.g. the dependency group name to the cache key.

Actual behavior:

I believe I can only add filepaths to the cache option of cache-dependency-path to control the cache key.

@fredrikaverpil fredrikaverpil added bug Something isn't working needs triage labels Sep 21, 2022
@panticmilos
Copy link
Contributor

hi @fredrikaverpil thank you for the report, we will take a look.

@panticmilos panticmilos self-assigned this Oct 5, 2022
@fredrikaverpil
Copy link
Author

@panticmilos I see this got labeled as bug and you self-assigned this recently. Do you have anything to share on this topic yet?
I'm thinking whether I should use the cache-dependency-path to work around this problem or if you have something else in mind? 😄

@panticmilos
Copy link
Contributor

Hi @fredrikaverpil,

First of all, sorry for the delayed response, and thank you for your patience 🙏

We have discussed your issue with our team and this is the conclusion. I don’t think we can utilize cache-dependency-path here since all of the dependencies groups are in the same file at the end. If there was some poetry command which would tell you which dependency group is installed, then it would make sense to implement such a thing. Otherwise, we would need one more input for it, which would make the solution the same as for actions/cache with an expanded primary key with a dependency group, and therefore we don’t see the need to implement such a thing, since we want to keep cache input as minimal as possible for setup actions.

I will close this issue now, but we can continue the conversation. Also, feel free to reopen it any time

@fredrikaverpil
Copy link
Author

fredrikaverpil commented Oct 10, 2022

@panticmilos I think this makes a lot of sense 👍

How about adding a note about this in your docs though?

For others who may stumble across this thread; a custom action supporting caching of poetry dependency groups:

# action.yml

name: poetry-install-with-caching
description: Poetry install with support for caching of dependency groups.

inputs:
  python-version:
    description: Python version, supporting MAJOR.MINOR only
    required: true

  poetry-version:
    description: Poetry version
    required: true

  install-command:
    description: Command run for installing dependencies
    required: false
    default: poetry install

  cache-key:
    description: Cache key to use for manual handling of caching
    required: true

  working-directory:
    description: Directory to run install-command in
    required: false
    default: ""

runs:
  using: composite
  steps:
    - uses: actions/setup-python@v4
      with:
        python-version: ${{ inputs.python-version }}

    - uses: actions/cache@v3
      id: cache-pip
      env:
        SEGMENT_DOWNLOAD_TIMEOUT_MIN: "15"
      with:
        path: |
          ~/.cache/pip
        key: pip-${{ runner.os }}-${{ runner.arch }}-py-${{ inputs.python-version }}

    - run: pipx install poetry==${{ inputs.poetry-version }} --python python${{ inputs.python-version }}
      shell: bash

    - uses: actions/cache@v3
      id: cache-poetry
      env:
        SEGMENT_DOWNLOAD_TIMEOUT_MIN: "15"
      with:
        path: |
          ~/.cache/pypoetry/virtualenvs
          ~/.cache/pypoetry/cache
          ~/.cache/pypoetry/artifacts
        key: poetry-${{ runner.os }}-${{ runner.arch }}-py-${{ inputs.python-version }}-poetry-${{ inputs.poetry-version }}-${{ inputs.cache-key }}-${{ hashFiles('poetry.lock') }}

    - run: ${{ inputs.install-command }}
      working-directory: ${{ inputs.working-directory }}
      shell: bash

eyurtsev added a commit to langchain-ai/langchain that referenced this issue May 10, 2023
# Add action to test with all dependencies installed

PR adds a custom action for setting up poetry that allows specifying a
cache key:
actions/setup-python#505 (comment)

This makes it possible to run 2 types of unit tests: 

(1) unit tests with only core dependencies
(2) unit tests with extended dependencies (e.g., those that rely on an
optional pdf parsing library)


As part of this PR, we're moving some pdf parsing tests into the
unit-tests section and making sure that these unit tests get executed
when running with extended dependencies.
jpzhangvincent pushed a commit to jpzhangvincent/langchain that referenced this issue May 12, 2023
# Add action to test with all dependencies installed

PR adds a custom action for setting up poetry that allows specifying a
cache key:
actions/setup-python#505 (comment)

This makes it possible to run 2 types of unit tests: 

(1) unit tests with only core dependencies
(2) unit tests with extended dependencies (e.g., those that rely on an
optional pdf parsing library)


As part of this PR, we're moving some pdf parsing tests into the
unit-tests section and making sure that these unit tests get executed
when running with extended dependencies.
EandrewJones pushed a commit to Oogway-Technologies/langchain that referenced this issue May 12, 2023
# Add action to test with all dependencies installed

PR adds a custom action for setting up poetry that allows specifying a
cache key:
actions/setup-python#505 (comment)

This makes it possible to run 2 types of unit tests: 

(1) unit tests with only core dependencies
(2) unit tests with extended dependencies (e.g., those that rely on an
optional pdf parsing library)


As part of this PR, we're moving some pdf parsing tests into the
unit-tests section and making sure that these unit tests get executed
when running with extended dependencies.
etianen added a commit to etianen/logot that referenced this issue Feb 11, 2024
These are actually wasting time triggering uninstalls from the cached
venv.

```
Installing dependencies from lock file

Package operations: 1 install, 0 updates, 4 removals

  • Removing iniconfig (2.0.0)
  • Removing loguru (0.7.2)
  • Removing pluggy (1.4.0)
  • Removing pytest (8.0.0)
  • Installing coverage (7.4.1)

Installing the current project: logot (0.4.0)
```

See actions/setup-python#582,
actions/setup-python#505
rytilahti pushed a commit to python-kasa/python-kasa that referenced this issue Mar 15, 2024
Caching pre-commit halves the linting time and the `action/setup-python`
cache does not handle `--extras` [properly
](actions/setup-python#505) so switching to
action/cache for the poetry cache
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants