Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update env vars in rocks #61

Merged
merged 4 commits into from
Nov 1, 2023
Merged

Update env vars in rocks #61

merged 4 commits into from
Nov 1, 2023

Conversation

i-chvets
Copy link
Contributor

Description

There was an issue discovered with latest ROCKs that incorrectly stored notebooks: canonical/notebook-operators#312

In order to resolve the issue environment variable HOME needs to be set properly.

Summary of changes:

  • Set HOME environment variable and sync it with working-dir
  • Updated version field to include dashes because of recent Rockcraft changes.

Ivan Chvets added 4 commits October 4, 2023 15:00
fix: update command and env variables
c65ed5e 
#54

Summary of changes:
- Updated environments in all Jupyter notebook server ROCKs to omit
  envirtonmen variables that need to be set in the Pod environment.
Added comment indicating this.
- Updated command to expand environment variables.
fix: added tests for env variables
43d17c8
Merge branch 'track/1.7' of github.com:canonical/kubeflow-rocks into …
94c115b 
…update-env-vars-in-rocks
fix: home env variable setting
8916c74 
Summary of changes:
- Set HOME environment variable and sync it with working-dir
- Updated version field to include dashes.
@i-chvets i-chvets requested a review from a team as a code owner October 30, 2023 15:42
@NohaIhab
Copy link
Contributor

NohaIhab commented Oct 30, 2023 •
edited
Loading

Hi @i-chvets
I tried out the jupyter-scipy rock but I still don't see the working directory as expected, and files do not persist when stopping the notebook
I published it to charmedkubeflow/jupyter-scipy-test:v1.7.0-20.04-1 if you want to play around with it.

@i-chvets
Copy link
Contributor Author

i-chvets commented Oct 31, 2023 •
edited
Loading

Tested with the test ROCK image pushed to registry: https://hub.docker.com/r/charmedkubeflow/jupyter-pytorch-full-test/tags

Screenshot from 2023-10-31 10-32-46

@NohaIhab Could you verify on your end with the above ROCK image.

@SamirPS
Copy link

SamirPS commented Nov 1, 2023

Hello

With @i-chvets image, I have the good PWD, but with @NohaIhab image, I have the wrong PWD. Noha, can you use rockcraft clean before rebuilding the image?

@NohaIhab
Copy link
Contributor

NohaIhab commented Nov 1, 2023

Confirming the above, the published rock charmedkubeflow/jupyter-pytorch-full-test:v1.7.0-20.04-1 has the right pwd and the files persist when stopping and starting the notebook.
Just a small comment, the user in a terminal created from a notebook is not the same as upstream, but I wouldn't block on this. I don't see it affecting the functionality.

@i-chvets i-chvets merged commit ad4e002 into track/1.7 Nov 1, 2023
@i-chvets i-chvets mentioned this pull request Nov 1, 2023
Merged
@kimwnasptd kimwnasptd deleted the update-env-vars-in-rocks branch November 16, 2023 15:33
@kimwnasptd kimwnasptd mentioned this pull request Nov 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NohaIhab NohaIhab NohaIhab approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@i-chvets @NohaIhab @SamirPS