snok / install-poetry

Github action for installing and configuring Poetry
MIT License
584 stars 53 forks source link

`poetry install` stalls on Windows with warm cache #18

Closed amotl closed 3 years ago

amotl commented 3 years ago

Dear Sondre,

thanks a stack for conceiving this excellent GitHub Action.

We already pinged you within https://github.com/earthobservations/wetterdienst/pull/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 https://github.com/earthobservations/wetterdienst/pull/331 in order to give us a chance to investigate more closely.

The observation is:

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 https://github.com/earthobservations/wetterdienst/commit/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 commented 3 years ago

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 commented 3 years ago

Hi again,

I just wanted to inform you about https://github.com/actions/cache/issues/175, which I summarized at https://github.com/earthobservations/wetterdienst/issues/332.

@webknjaz and @pradyunsg added valuable comments at https://github.com/actions/cache/issues/175#issuecomment-636704469 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 https://github.com/stellargraph/stellargraph/pull/1720.

With kind regards, Andreas.

sondrelg commented 3 years ago

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.