-
Notifications
You must be signed in to change notification settings - Fork 305
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
Add . to the PYTHONPATH in ImageSpec #2447
Conversation
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For custom base images, should ImageSpec
change the WORKDIR
to /root
?
@@ -91,7 +91,7 @@ def create_envd_config(image_spec: ImageSpec) -> str: | |||
run_commands = _create_str_from_package_list(image_spec.commands) | |||
conda_channels = _create_str_from_package_list(image_spec.conda_channels) | |||
apt_packages = _create_str_from_package_list(image_spec.apt_packages) | |||
env = {"PYTHONPATH": "/root", _F_IMG_ID: image_spec.image_name()} | |||
env = {"PYTHONPATH": "/root:.", _F_IMG_ID: image_spec.image_name()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "." is easy to miss. I think using PWD
works too?
env = {"PYTHONPATH": "/root:.", _F_IMG_ID: image_spec.image_name()} | |
env = {"PYTHONPATH": "/root:$PWD", _F_IMG_ID: image_spec.image_name()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need .
you just need the :
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does this work? I was under the impression that the current working directory was already part of sys.path
when the python interpreter starts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For custom base images, should ImageSpec change the WORKDIR to /root?
we don't need to. It should use the workdir in the base image.
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Can we add a description of the problem to this PR? I'd love to understand why this fixes the problem. |
Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Jan Fiedler <[email protected]>
Tracking issue
https://flyte-org.slack.com/archives/CP2HDHKE1/p1716911881652929
Why are the changes needed?
Module not found while running on a remote cluster since Python can not find the local module unless you explicitly add the local directory to the
PYTHONPATH
Note: when you are using non-default
base_image
, theWORKDIR
might be others (/
or/home
, etc).What changes were proposed in this pull request?
Add . to PYTHONPATH
How was this patch tested?
Setup process
Screenshots
Check all the applicable boxes
Related PRs
NA
Docs link
NA