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 install stalls on Windows with warm cache #18

Closed
amotl opened this issue Jan 24, 2021 · 7 comments · Fixed by #21
Closed

poetry install stalls on Windows with warm cache #18

amotl opened this issue Jan 24, 2021 · 7 comments · Fixed by #21

Comments

@amotl
Copy link

amotl commented Jan 24, 2021

Dear Sondre,

thanks a stack for conceiving this excellent GitHub Action.

We already pinged you within earthobservations/wetterdienst#328 yesterday, where @gutzbenj was about to switch from Gr1N/setup-poetry to snok/install-poetry. However, he was still struggling with it so he was about to move on to abatilo/actions-poetry already.

Now that I've recognized that your GitHub Action probably has solved some problems specifically related to Windows environments already, I wanted to take the chance and created earthobservations/wetterdienst#331 in order to give us a chance to investigate more closely.

The observation is:

  • When running on Linux and macOS, everything works successfully on all occasions [1].
  • When running on Windows, the job succeeds when being on a cold cache [1].
  • When running on Windows, the job completely stalls on poetry install when running on a warm cache [2].

Maybe you already have an idea what might be going wrong there? Otherwise, I kindly ask for your support here - we really would like to see that download/environment caching also works appropriately on Windows.

With kind regards,
Andreas.

[1] https://github.com/earthobservations/wetterdienst/actions/runs/507426192
[2] https://github.com/earthobservations/wetterdienst/actions/runs/507437925

@sondrelg
Copy link
Member

Hi Andreas!

I remember having the same issue when setting up the action, and I don't think I ever managed to solve it. It's a very weird issue, and stalled workflows are very hard to debug 😅

In other words, I don't really know why the cache stalls the workflow, unfortunately.

If you manage to get it working with another workflow, please let me know; and if you don't I could maybe have another go at making it work sometime in the next couple of days.

@amotl
Copy link
Author

amotl commented Jan 24, 2021

Hi Sondre,

thanks for confirming our observations. I took the chance to have a short glimpse at Poetry's issue tracker and found those items to be related to indefinite hangs/stalls/freezes:

However, I did not investigate in more detail yet.

With kind regards,
Andreas.

@sondrelg
Copy link
Member

sondrelg commented Jan 24, 2021

I'll have a look at this, thanks!

Just in case this is useful to you - have you considered trying to drop the cache step from just the Windows run?

Without checking I can't be sure it's possible, but perhaps something like this could work:

 - name: Install dependencies
   if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true' and runner.os != 'Windows'
   run: poetry install --no-interaction --no-root --extras=http --extras=sql --extras=excel

@amotl
Copy link
Author

amotl commented Jan 24, 2021

We had everything working perfectly since months and our endeavor right now was actually about making Poetry environment caching also work on Windows, which didn't happen before. Apparently, this is a holy grail ;].

After adding earthobservations/wetterdienst@6d4b13f, the process just freezes one step before. So, when touching a Poetry environment after it had been restored from the cache, things go south.

Side note: Recently, when working on introducing actions/cache on another repository, I discovered a comment that caching the whole virtualenv can lead to obscure side effects and it was recommended against doing it at all. So, caching would only be applied to the downloaded artifacts but they would still be installed into a virtualenv newly created each time.
However, I am not able to find back this comment, so please don't take it for granted - I just wanted to mention it.

@sondrelg
Copy link
Member

That's interesting, so perhaps caching the wheels is a better approach for Windows.

I've re-triggered the problem here https://github.com/snok/install-poetry/runs/1757673921?check_suite_focus=true and things are starting to come back to me 😅 Debugging a process with no output is tricky.

I'll try a few combinations of caching, and hopefully we'll be able to find something that works 🤞

@amotl
Copy link
Author

amotl commented Jan 24, 2021

Hi again,

I just wanted to inform you about actions/cache#175, which I summarized at earthobservations/wetterdienst#332.

@webknjaz and @pradyunsg added valuable comments at actions/cache#175 (comment) ff. why only pip's wheel cache should be cached between subsequent invocations and that neither site-packages nor entire interpreter trees should be cached at all.

Otherwise, you might want to at least add a note to the documentation that - while this approach gains better performance - it has fragile aspects as outlined by @huonw at stellargraph/stellargraph#1720.

With kind regards,
Andreas.

@sondrelg
Copy link
Member

After reading all the issues and reflecting a little I think I will add a dedicated section to the README informing about the stalling that happens when you attempt to cache your venv on Windows, and point to caching your pip wheels as an alternative option. I was thinking about maybe making a full blown example with ifs conditionally caching one way on UNIX, and another on Windows, but I think I will go with just the mention for now.

If anyone reading this wants to make a contribution with test cases and docs, that's of course always welcome 👏

In general though, the cache problem seems like a Poetry and/or cache issue manifesting itself here, rather than it being an error in the poetry-install implementation, so until proven otherwise I think I will label this out of scope.

Thanks again Andreas for the report 👍 Good luck further speeding up your workflows.

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 a pull request may close this issue.

2 participants