-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Clean up runtime code for finding/using standalone python and entering a venv #26012
Comments
I'll mention that there is a
I wonder if we should first run the activate script upon entering, then parsing |
Some more info: The server must be launched on Linux/OSX (assumes tarball unpacked at
Windows works less well, but can be made to work. The server must by launched (assumes zip unpacked at
|
@jacksonrnewhouse found that updating PATH to include That leaves Windows:
|
#25969 lists this as a followup once it is merged:
influxdb3/src/main.rs
has some hacks to help find things. This is clearly not the right place for this code and there may be ways to not need it at all. Specifically:set_pythonhome()
setsPYTHONHOME
based on where it finds the runtime. I kinda feel like this shouldn't be needed, but the concept of it isn't terrible (even if the location in the code and probably the code itself is)set_pythonpath()
setsPYTHONPATH
and is required on Windows for some reason. This should be fixed properlyinfluxdb3_processing_engine/src/virtualenv.rs
has a few changes (but not enough):get_python_version()
is adjusted to usepython
on Windows aspython3
doesn't exist thereinitialize_venv()
is adjusted to useScripts/activate
and callcmd.exe
on Windows--virtual-env-location
doesn't work on any platform. Linux and OSX can be made to work with venv usingsource /path/to/venv/bin/activate
and launchinginfluxdb3 serve ...
under it (client doesn't need this). Windows needs to setPYTHONPATH=\path\to\venv\Lib\site-packages
Tentatively assigning to @jacksonrnewhouse for (at least) the
virtualenv.rs
and--virtual-env-location
. I can handle the move away frommain.rs
but I suspect @jacksonrnewhouse is best positioned for this.The text was updated successfully, but these errors were encountered: