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

Add poetry caching support #281

Merged
merged 15 commits into from Mar 31, 2022
Merged

Conversation

patrick91
Copy link
Contributor

@patrick91 patrick91 commented Nov 24, 2021

Description:
A continuation of #266 to add support for caching poetry's virtualenv

This is still draft, I need to implement the actual caching, but it is good to start running the tests early :)

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

__tests__/cache-restore.test.ts Show resolved Hide resolved
src/cache-distributions/poetry-cache.ts Outdated Show resolved Hide resolved
.github/workflows/e2e-cache.yml Outdated Show resolved Hide resolved
@patrick91 patrick91 marked this pull request as ready for review Nov 27, 2021
@patrick91
Copy link
Contributor Author

@patrick91 patrick91 commented Nov 27, 2021

@floatingpurr
Copy link

@floatingpurr floatingpurr commented Dec 22, 2021

Great improvement!

@dimaqq
Copy link

@dimaqq dimaqq commented Jan 20, 2022

Keep going! :)

@Zxilly
Copy link

@Zxilly Zxilly commented Jan 30, 2022

Any update on this?

@patrick91
Copy link
Contributor Author

@patrick91 patrick91 commented Jan 31, 2022

Any update on this?

I've rebased the PR, but we have to wait the mainteners to review the PR. They are probably busy with other priorities so we'll probably have to wait a bit 😊

.github/workflows/e2e-cache.yml Outdated Show resolved Hide resolved
__tests__/data/.tool-versions Outdated Show resolved Hide resolved
src/cache-distributions/poetry-cache.ts Outdated Show resolved Hide resolved
src/cache-distributions/poetry-cache.ts Outdated Show resolved Hide resolved
@dmitry-shibanov
Copy link
Contributor

@dmitry-shibanov dmitry-shibanov commented Feb 18, 2022

Could you please sync with the main branch ?

@patrick91
Copy link
Contributor Author

@patrick91 patrick91 commented Feb 18, 2022

I closed this by mistake :) @dmitry-shibanov the PR should be up to date now!

@patrick91 patrick91 reopened this Feb 18, 2022
@dmitry-shibanov
Copy link
Contributor

@dmitry-shibanov dmitry-shibanov commented Feb 21, 2022

Hello @patrick91. Could you please run these commands:

  • npm run build
  • npm run format

@patrick91
Copy link
Contributor Author

@patrick91 patrick91 commented Feb 21, 2022

@dmitry-shibanov done!

@patrick91
Copy link
Contributor Author

@patrick91 patrick91 commented Feb 25, 2022

Not 100% sure why the CI failied, I'll try to take a look soon, maybe a rebase helps 😊

@patrick91 patrick91 force-pushed the feature/poetry-caching branch 2 times, most recently from 57c8ee5 to a580a2d Compare Feb 26, 2022
@patrick91
Copy link
Contributor Author

@patrick91 patrick91 commented Mar 7, 2022

Rebased this again :)

@patrick91 patrick91 force-pushed the feature/poetry-caching branch from a580a2d to 86304bc Compare Mar 7, 2022
@m0ar
Copy link

@m0ar m0ar commented Mar 9, 2022

Super stoked for this! Keep the good work up @patrick91 💘

@rc-mattschwager
Copy link

@rc-mattschwager rc-mattschwager commented Mar 25, 2022

Friendly bump here, can a maintainer at least approve the CI workflows so the submitter can see if everything's still passing?

First-time contributors need a maintainer to approve running workflows.

@patrick91 patrick91 requested a review from as a code owner Mar 26, 2022
@patrick91
Copy link
Contributor Author

@patrick91 patrick91 commented Mar 29, 2022

@dmitry-shibanov pipenv seems to fail on main too, shall I sent a PR to try and fix that?

Also I did rebase and ran npm run build && npm run format, but the check is still failing 🤔

image

@brcrista
Copy link
Contributor

@brcrista brcrista commented Mar 30, 2022

@patrick91 I looked at that failure briefly but couldn't figure it out. If you know what the issue is and want to submit a separate PR, that would be great. Otherwise, we don't need to block this on that.

What you do need to fix is the check-dist failure. You can just download the artifact from that run and check it in.

@patrick91
Copy link
Contributor Author

@patrick91 patrick91 commented Mar 30, 2022

@brcrista done 😊

patrick91 and others added 2 commits Mar 30, 2022
Co-authored-by: Brian Cristante <33549821+brcrista@users.noreply.github.com>
Copy link
Contributor

@brcrista brcrista left a comment

Thank you @patrick91!

@brcrista brcrista merged commit 6c56602 into actions:main Mar 31, 2022
84 of 96 checks passed
@Zxilly
Copy link

@Zxilly Zxilly commented Mar 31, 2022

nice work

@patrick91 patrick91 deleted the feature/poetry-caching branch Mar 31, 2022
@m0ar
Copy link

@m0ar m0ar commented Apr 1, 2022

@patrick91 Thank you! 🧁 💘

@tiwarishub tiwarishub mentioned this pull request Apr 1, 2022
2 tasks
tony added a commit to vcs-python/libvcs that referenced this issue Apr 2, 2022
As of yesterday, `@actions/setup-python@v3` supports this via v3.1.0: [Release](https://github.com/actions/setup-python/releases/tag/v3.1.0), [Tag](https://github.com/actions/setup-python/tree/v3.1.0), actions/setup-python#281

```yaml
steps:
- uses: actions/checkout@v3
- name: Install poetry
  run: pipx install poetry
- uses: actions/setup-python@v3
  with:
    python-version: '3.9'
    cache: 'poetry'
- run: poetry install
- run: poetry run pytest
```

 See also: python-poetry/poetry#4205
tony added a commit to vcs-python/libvcs that referenced this issue Apr 2, 2022
…ython@v3

poetry install will pick up the python binary at the time it was
installed (before actions/python ran) so it looks like poetry env needs
to be set manually.

https://github.com/actions/setup-python/releases/tag/v3.1.0
https://github.com/actions/setup-python/tree/v3.1.0
actions/setup-python#281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants