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.

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

fix: enable running image on Renku #1105

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

olevski
Copy link
Contributor

@olevski olevski commented Jan 21, 2025

This enables running the image on Renku.

See the qe-test-tasko session launcher on https://renkulab.io/v2/projects/jusong.yeu/aiidalab-qe-app.

The session launcher has the following specs:
Screenshot From 2025-01-21 13-45-02

@superstar54 superstar54 self-requested a review January 21, 2025 12:53
@superstar54
Copy link
Member

Hi @olevski , thanks for the fix.

I got this error when visit the link ( https://renkulab.io/v2/projects/jusong.yeu/aiidalab-qe-app.) you shared

image

@superstar54 superstar54 requested a review from unkcpz January 21, 2025 12:55
@superstar54
Copy link
Member

From the name, seems the link (jusong.yeu/aiidalab-qe-app) is related to @unkcpz

@olevski
Copy link
Contributor Author

olevski commented Jan 21, 2025

@superstar54 this project is owned by Jason and it is not public. That is why you get a 404 when you visit the link. He would have to add you to it to test. I was emailing with Jason back and forth. It is probably best that he reviews and tests this. I will email him and let him know and see how he wants to progress this.

Copy link
Member

@superstar54 superstar54 left a comment

Choose a reason for hiding this comment

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

The changes LGTM, Thanks!

@superstar54
Copy link
Member

Yes, I have added @unkcpz as a reviewer.

Copy link

codecov bot commented Jan 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.94%. Comparing base (d5a1aec) to head (41bfb23).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1105      +/-   ##
==========================================
+ Coverage   70.92%   70.94%   +0.01%     
==========================================
  Files         115      115              
  Lines        7158     7158              
==========================================
+ Hits         5077     5078       +1     
+ Misses       2081     2080       -1     
Flag Coverage Δ
python-3.11 70.92% <ø> (ø)
python-3.9 70.94% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@olevski
Copy link
Contributor Author

olevski commented Jan 21, 2025

Ok cool. I did not know Jason's handle. I will email him as well.

@unkcpz ping!

cd /home/${NB_USER} && tar -cf /opt/conda/home.tar .
# NOTE: The work folder is empty but if included clashes with the work folder in a Renku
# session whose permissions cannot be changed.
cd /home/${NB_USER} && tar -cf /opt/conda/home.tar --exclude work .
Copy link
Member

Choose a reason for hiding this comment

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

Okay, we include the work from legacy jupyter, it is better to exclude it from the tar.

@unkcpz
Copy link
Member

unkcpz commented Jan 21, 2025

Thanks @olevski, when we start the aiidalab-qe on k8s, I also pass

        extraEnv:
            JUPYTERHUB_SINGLEUSER_APP: notebook.notebookapp.NotebookApp

to the jupyterhub to start not using the lab but the notebook mode. How to pass this to the renku configuration?

Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

Not sure where to make it by default the notebookapp mode, maybe it require to set the argument in the renku side before the container start?
I am trying:

Args: ["sh","-c","bash start.sh jupyter notebook --NotebookApp.ip=0.0.0.0 --NotebookApp.port=8888 --NotebookApp.base_url=$RENKU_BASE_URL_PATH --NotebookApp.token=\"\" --NotebookApp.password=\"\" --NotebookApp.allow_remote_access=true --ContentsManager.allow_hidden=true --NotebookApp.allow_origin=*"] 

but it still start in the jupyterlab mode.

@olevski
Copy link
Contributor Author

olevski commented Jan 21, 2025

I am not sure what it's supposed to look like. Is it this?
Screenshot From 2025-01-21 16-25-06

@unkcpz
Copy link
Member

unkcpz commented Jan 21, 2025

I am not sure what it's supposed to look like. Is it this?

yes, exactly! I'll check what you changed in the setup. I think for the changes here it should all good.

@olevski
Copy link
Contributor Author

olevski commented Jan 21, 2025

@unkcpz the only thing I changed is the Default URL path it went from /lab to /. This controls what path will be added to the end of the full session path when the renku session is opened.

If you open the /lab path by default then you get the ui I originally showed. The changes you made with swapping all arguments from ServerApp to NotebookApp are not needed.

Also @unkcpz I see that you are changing the session launcher I created. Also while I was editing it. So I am pretty sure now they are all in a weird state.

@unkcpz
Copy link
Member

unkcpz commented Jan 21, 2025

Also @unkcpz I see that you are changing the session launcher I created. Also while I was editing it. So I am pretty sure now they are all in a weird state.

Sorry, I was changing the url to open the home app directly. I think it works as I expected exactly, and I am able to manage more issues if there show up. I'll summarize and document it somewhere later. Thanks a lot for the help!!

@olevski
Copy link
Contributor Author

olevski commented Jan 21, 2025

Ok great! I leave it up to you to click merge on this PR or redo it or whatever you want to do with it.

@unkcpz unkcpz merged commit dd54da6 into aiidalab:main Jan 21, 2025
8 checks passed
@olevski
Copy link
Contributor Author

olevski commented Jan 21, 2025

@unkcpz just for completeness this is enough to open that notebook in the mode you wanted. But some more configuration on the jupyter side is needed in order for the app to fully run. It complains that it cannot find the kernel.

Screenshot From 2025-01-21 16-54-24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants