Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix issue #22 - conda-env-kernel-image example is broken #26

Merged
merged 7 commits into from
Nov 17, 2022
Merged

Fix issue #22 - conda-env-kernel-image example is broken #26

merged 7 commits into from
Nov 17, 2022

Conversation

eitansela
Copy link
Contributor

Issue #, if available:
#22

Description of changes:
Fix conda-env-kernel-image example is broken issue:

  • Update base conda environment instead of creating a new myenv one.
  • Fixed kernel name to be python3, as the one on Image.
  • Fixed MountPath to be /home/sagemaker-user.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@knaresh knaresh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making the change

Comment on lines 10 to 13
"FileSystemConfig": {
"MountPath": "/root",
"MountPath": "/home/sagemaker-user",
"DefaultUid": 0,
"DefaultGid": 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.aws.amazon.com/sagemaker/latest/APIReference/API_FileSystemConfig.html

why change MounPath. It appears user inside the container is still root

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you leave it root, than it will fail to mount.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with existing config and kerne launch succeeded without this change. can you drop this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted to "MountPath": "/root",, tested it on my Studio domain and it worked.
Pushed changes.

@@ -3,12 +3,12 @@
"KernelGatewayImageConfig": {
"KernelSpecs": [
{
"Name": "conda-env-myenv-py",
"Name": "python3",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super confused, how did it ever work before

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It must be python3 so this is a good question...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got similar working correctly this week (conda-env-mycoolenv-py) - I believe this is the auto-detection mentioned in "Kernel discovery" section of the custom image spec? I logged docs feedback that they should clarify there what the "Name"s are for auto-detected conda environments.

I believe this is something that SageMaker or jupyter_kernel_gateway is doing? Because as discussed here seems like auto-setup of conda envs as Jupyter kernels is not automatic from conda itself?

RUN conda env create -f environment.yml
RUN conda env update -f environment.yml --prune
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sry missed this earlier. Why do we need this change? what is prune trying to achieve?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the original sample, this iwll create a new conda environment called myenv. Then, since base environment is being selected by default in Studio, so how do you switch to myenv conda?
With this change, I override the base environment, which is selected by default by Studio.

@knaresh knaresh merged commit 4c0b75e into aws-samples:main Nov 17, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants