fix: Make file upload work when waved run separately #2225 #2232
Conversation
There was a problem hiding this comment.
Looking good @marek-mihok! Can we do without external lib though?
py/h2o_wave/h2o_wave/cli.py
Outdated
| print('Could not connect to Wave server. Please start the Wave server (waved or waved.exe) prior to running any app.') | ||
| return | ||
|
|
||
| if not os.environ.get('H2O_WAVE_WAVED_DIR') and is_waved_present: | ||
| os.environ['H2O_WAVE_WAVED_DIR'] = sys.exec_prefix | ||
| os.environ['H2O_WAVE_WAVED_DIR'] = waved_cwd |
There was a problem hiding this comment.
If this works reliably, we can deprecate H2O_WAVE_WAVED_DIR. Add a TODO here for now.
py/h2o_wave/h2o_wave/cli.py
Outdated
| waved_cwd = psutil.Process(proc.pid).cwd() | ||
| except psutil.NoSuchProcess: | ||
| pass | ||
| waved_path = os.path.join(waved_cwd, proc_name) |
There was a problem hiding this comment.
This is only true for platform specific wheel files. Platform-agnostic whl does not contain waved binary.
@mturoci my research shows there is no pythonic (or other simple) way of getting process cwd by its name on Windows. Libs like Maybe we could include |
In that case, you can either implement for UNIX systems only and defer to HTTP on err and Windows or go for HTTP everywhere and keep using |
|
Edit: Let's just go for HTTP uploads when waved is run separately and local uploads when waved is started automatically. This would be the safest and simplest option. |
|
Added a small refactor. @marek-mihok please verify if works. If it does, good to merge. Thanks! |
|
@mturoci works, it's good to go. |
The PR fulfills these requirements: (check all the apply)
mainbranch.feat: Add a button #xxx, where "xxx" is the issue number).Closes #xxx, where "xxx" is the issue number.uifolder, unit tests (make test) still pass.This PR fixes the issue when
H2O_WAVE_WAVED_DIRhas a wrong value in some scenarios. The fix is accomplished by finding the PID of thewaved/waved.exeand getting its CWD which is then set toH2O_WAVE_WAVED_DIRenv variable.Successfully tested on both Mac and Windows.
Closes #2225