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

FIX: Github action VENV management to prevent the bug in #1457, python 3.8 fundamental depreciation #1469

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

daquinteroflex
Copy link
Collaborator

@daquinteroflex daquinteroflex commented Feb 14, 2024

Trying to sort out the github action issue in #1457 without bothering everyone.

I couldn't reproduce the error you were getting so I thought to fix the github action on top of it which I thought just involved minor action version updates (but did not). I accidentally closed the PR when I tried to reset back to the original state. I have put everything back in it's place as it was originally in this PR #1470 . However, the reason the actions were not passing had to do with some github caching in the action. The same PR which only contains your commit in the latest state, passes the tests. That’s why I couldn’t reproduce it originally.

What I should have done was to branch out and make another PR to fix the github action separately - which is what I've done now. The weird thing is that we can see I haven't changed anything, and yet the github action is working fine. But I think I found out why the error was happening which is related to the VENV caching and am working to fix it from happening again.

The error that made the github actions fail was:

Run poetry install --no-interaction -E dev

Command ['D:\\a\\tidy3d\\tidy3d\\.venv\\Scripts\\python.exe', '-I', '-W', 'ignore', '-c', 'import sys\n\nif hasattr(sys, "real_prefix"):\n    print(sys.real_prefix)\nelif hasattr(sys, "base_prefix"):\n    print(sys.base_prefix)\nelse:\n    print(sys.prefix)\n'] errored with the following return code 103

Error output:
No Python at '"C:\hostedtoolcache\windows\Python\3.11.7\x64\python.exe'

This PR is actually a pretty strong rewrite of the way the github action used to work:

  • Will prevent the above bug and the one below:
  • The way that the poetry installation is managed is restructured. More intuitive to terminal
  • It will take longer to run each time at the expense of not storing venvs for each poetry.lock individually
  • Doing this debug raised that the old action was modifying the venv for all the python 3.8 installations for each platforms. This was bad. The reason is that this meant it was internally updating the venv to 3.9 because it is fundamentally not possible to resolve a poetry.lock for python 3.8 because there are no supported jax for 3.8. This does not concern the extra jax, I refer to the full environment management compiled in poetry.lock cannot even be created for python 3.8 since there is no resolved envrionment for a given set of platforms. I don't think it's possible to say that python 3.8 is supported in the pyproject.toml just for ubuntu and linux.

Unfortunately I think we fundamentally need to depreciate python 3.8. It probably wasn't even working before on Windows but the venv caching hid that from us. It is not possible to generate a resolved poetry.lock environment, independent of the extras installation mechanism. Like in any case any 3.8 installation is broken in windows at some level, and not being able to generate the poetry.lock environment is just trying to avoid us from users dealing with that.

@daquinteroflex daquinteroflex changed the title Dario/fix GitHub actions FIX: Github action VENV management to prevent the issue in #1450 Feb 14, 2024
@daquinteroflex daquinteroflex changed the title FIX: Github action VENV management to prevent the issue in #1450 FIX: Github action VENV management to prevent the issue in #1457 Feb 14, 2024
@daquinteroflex daquinteroflex changed the title FIX: Github action VENV management to prevent the issue in #1457 FIX: Github action VENV management to prevent the bug in #1457 Feb 14, 2024
@daquinteroflex daquinteroflex force-pushed the dario/fix_github_actions branch 3 times, most recently from d6c0be3 to 495f78a Compare February 14, 2024 13:05
@daquinteroflex daquinteroflex changed the title FIX: Github action VENV management to prevent the bug in #1457 FIX: Github action VENV management to prevent the bug in #1457, python 3.8 fundamental depreciation Feb 14, 2024
@daquinteroflex daquinteroflex merged commit 35c8514 into pre/2.6 Feb 14, 2024
16 checks passed
@daquinteroflex daquinteroflex deleted the dario/fix_github_actions branch February 14, 2024 14:43
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

2 participants