radical-cybertools / radical.pilot

RADICAL-Pilot
http://radical-cybertools.github.io/radical-pilot/index.html
Other
54 stars 23 forks source link

move from custom virtualenv version to `venv` module #2847

Closed andre-merzky closed 1 year ago

andre-merzky commented 1 year ago

We have relied on a specific virtualenv version for pilot bootstrapping for a long time. That worked well as it gave us a stable virtualenv stack to work with. Now that version has fallen too far behind stock python (see #2845) and it's time to revisit that approach. This PR switches back to the default venv module which has become quite stable over the last releases. This PR though should wait for the matrix tests to kick in (see #2846) before being considered for merge.

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 40.19%. Comparing base (d837940) to head (dce01c2). Report is 3382 commits behind head on devel.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## devel #2847 +/- ## ========================================== - Coverage 40.22% 40.19% -0.03% ========================================== Files 94 94 Lines 10268 10264 -4 ========================================== - Hits 4130 4126 -4 Misses 6138 6138 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

andre-merzky commented 1 year ago

every reference of virtenv_dist

Ah, of course! Thanks, that's cleaned up now.

andre-merzky commented 1 year ago

LGTM! just one last cleanup is an option description in bootstrapper

Man, am I glad that you review this! :-) Fixed!

andre-merzky commented 1 year ago

Alas, I encountered a number of Python deployments which don't have the venv installed by default. ARGH. So we have to revert this before release, or at least have the old virtenv fetching mechanism as fallback...

mtitov commented 1 year ago

@andre-merzky, I don't understand that, as I see it - venv is part of python since 3.5 (link), can you please expand on errors you've encountered?

eirrgang commented 1 year ago

@andre-merzky, I don't understand that, as I see it - venv is part of python since 3.5 (link), can you please expand on errors you've encountered?

The CPython distribution allows you to disable individual built-in packages. For some reason, Linux distros frequently do this, and, e.g. on Ubuntu, you have to explicitly install the python3-venv package, or the system python won't have the venv module. I don't remember seeing such problems with lmod / environment modules, but it is a common problem with the system python installation.

mtitov commented 1 year ago

The CPython distribution allows you to disable individual built-in packages. For some reason, Linux distros frequently do this, and, e.g. on Ubuntu, you have to explicitly install the python3-venv package, or the system python won't have the venv module. I don't remember seeing such problems with lmod / environment modules, but it is a common problem with the system python installation.

@eirrgang ah, I see, thank you Eric!