-
Notifications
You must be signed in to change notification settings - Fork 0
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
[patch] Debug py version #124
Conversation
Ok, so My guess is that one of the dependencies is not available at 3.9 (or not 3.12) and python gets updated alongside. |
Manually adding the unittest and coveralls-codacy dependencies still gives the correct python and python_abi versions. Time to kick it up a notch at try invoking |
Ok, the problem occurs when we What remains is to update |
The cache needs to know the path for the conda env it's looking for. When I run |
Ok, so here we hit the first and only real problem; |
It does use The minimal change would be if there were some way to know what the conda env path would be a-priori, then simply The incubator docs themselves for caching envs actually suggest to do exactly what we currently do, so that's no help.
The cache key doesn't depend on this, so my gut reaction is to break the cache into two pieces: first storing the env path at some fixed, known location, and then second storing the actual env. We can try restoring the location cache and only if the cache gets hit do we proceed to restore the actual cached env using the result of location cache for the |
I couldn't think of anything more elegant, so let's go with the double-cache attack. |
Aha, it's not finding the cached conda executable because And of course it doesn't, because I only cache |
I should be able to activate a conda env from anywhere, so let's try that with the built-in miniforge installation and see if it works. It would be simpler than caching/reinstalling mambaforge. |
Well, that's infuriating. I got the built-in conda executable to activate the |
GitHub doesn't do anything to manage conda env persistence between steps, much less jobs. The modifications to the conda config seem persistent enough for my purposes, but then one still needs to jump through hoops to activate the correct env in each step. Since the shell profile is not sourced between each step, there is no easy way to automate this (one could pass the buck to the shell profile, but then the user needs to remember to source that each step!) As far as I understand, Sadly, I think the most practical thing to do is give up on the speed of leveraging the built-in conda binary and just always run |
Uncaching, installing a blank miniconda, and then activating the uncached env works fine, but the activated env is not persistent across steps. Uncaching, and installing a miniconda that explicitly activates the existing env as an arg is persistent but the env is somehow completely empty -- not a single package! Unrelated but tested in parallel, stopping a clean miniconda from activating the base env brings the time down from 27s to 20s (N=1 test), so not mind blowing but worth doing if the opportunity presents itself. |
Super, creating an empty env then unpacking the cache into the same location worked perfectly! I get the right packages, it's all persistent to outside the called action, and it's faster than setting up the env from scratch (for this particular env ( With a better grasp of how this works, I think I can even streamline the process and eliminate the two-stage caching. |
Setting up the cache took 45s, exploiting it takes 35s, and running without any caching at all takes 30s. Again, all N=1 tests with the super simple env from Changing the env to be a bit more complex ( This is hardly an exhaustive or scientific test, but we reassuringly see that creating complete+caching takes longer than creating, and that creating empty+uncaching is faster than creating complete+caching. Whether creating complete is faster or slower than creating empty+uncaching depends on the complexity and size of the environment. Since the losses with caching are small for simple environments but the gains are significant for more complex environments, my qualitative conclusion is that caching should be the default behaviour. |
I want to put this branch through its paces exploiting it in a real test environment (especially across all the host OSs), then if that goes well I'll come back and clean this up and release it. |
Taking the new |
Looks OK on snippets, let's clean it up. |
There is an awkward edge case when the update env explicitly specifies a range of python versions that this can override the python version specified as an argument to `conda-incubator/setup-miniconda`; although the update approach is suggested in that action's docs for caching an env, the approach here is safer. A new input variable is introduced to store the location of the env, but the old `miniforge-activate-environment` can still be specified for backwards compatibility (although what happens under the hood has changed).
f475e47
to
7963349
Compare
Per #123, non-default (3.12) python version unit tests are not being run. Let's fix this.