Implement opt-in countdown feature and add to image recipe #525

edan-bainglass · 2025-04-01T16:20:01Z

Migrated from #524

This PR reimplements aiidalab-timer as a core feature of AiiDAlab instead of an app.

Jupyter provides entry points to customize notebook JS and CSS (see here for old, but as far as I can tell, still valid docs). Following this approach, the PR introduces under lab/countdown/ the following:

custom.js - a script implementing the countdown timer, applying it consistently throughout AiiDAlab
custom.css - styling the countdown-related DOM as a sticky banner above the native navbar

The PR also introduces a new "before_notebook" script, namely 70_prepare_countdown_config.sh that creates a config.json file in the custom folder:

{
  "ephemeral": # -> 1 if $LIFETIME is provided by deployment, 0 otherwise
  "lifetime": "$LIFETIME"
}

The PR updates the lab/Dockerfile to copy the necessary files to site-packages/notebook/static/custom/ and add the PYTHON_MINOR_VERSION environment variable required by the container-runtime script.

Container uptime

The countdown is based on the "uptime" of the container obtained via ps -p 1 -o etime=. The PR exposes this to JS via a new /uptime endpoint deployed via the aiidalab_uptime.py script, which is copied directly into site-packages. The endpoint is registered with Jupyter via the aiidalab_uptime.json config file, which is deployed by the Dockerfile to /opt/conda/etc/jupyter/jupyter_notebook_config.d.

To be considered

In some repos, we override etc/jupyter/jupyter_notebook_config.py directly (see demo-server for example). We may want to consider using the above approach to modify Jupyter's behavior in general. TBD. Pinging @unkcpz

edan-bainglass · 2025-04-02T07:22:21Z

@unkcpz the feature is now timezone-independent. However, it is a bit hacky. Maybe we can clean it up. Let's discuss in the office.

unkcpz · 2025-04-02T09:17:58Z

About the culling time setup I mentioned in the #524, I talk to @edan-bainglass off-line and we agree if it is the rigid 12h no matter user interact with notebook or not, which means the timer shows the left time exactly how long the pod or kernel will be shutdown. Then I think it is a correct implementation.

From the demo server deployment config in https://github.com/aiidalab/aiidalab-demo-server/blob/23f91a638588de9241dba8de6f905886efb5ddf4/basehub/values.yaml.j2#L100-L106. This apparently is the config for the jupyterhub helm chart, so I worried this set for the culling idle time? @yakutovicha can you double check? thanks.

The k8s when deploy can also set a rigid timeout for the pod which might be more reliable and you may want to use (https://kubernetes.io/docs/concepts/workloads/controllers/job/#job-termination-and-cleanup).

unkcpz

I still think these changes can only stay in the demo-server deployment as I did for the customized jupyterhub index page through k8s ConfigMaps. Then the hacky solution could be avoid.

unkcpz · 2025-04-02T09:18:55Z

stack/lab/Dockerfile

+ARG PYTHON_MINOR_VERSION
+ENV PYTHON_MINOR_VERSION=${PYTHON_MINOR_VERSION}
+COPY --chown=${NB_UID}:${NB_GID} countdown/ ${CONDA_DIR}/lib/python${PYTHON_MINOR_VERSION}/site-packages/notebook/static/custom/
+
+# Add endpoint to fetch container uptime
+COPY --chown=${NB_UID}:${NB_GID} aiidalab_uptime.py ${CONDA_DIR}/lib/python${PYTHON_MINOR_VERSION}/site-packages/
+RUN echo "c.NotebookApp.server_extensions.append('aiidalab_uptime')" >> /etc/jupyter/jupyter_notebook_config.py


As mentioned in the review of #524, can you move this part next to the logo.png copy? we move these lines here when we debug together and we found the problem was the env variable not properly set.
Keep the PYTHON_MINOR_VERSION set to where it used would be easier for future maintenance.

The echo and pipeline to jupyter config looks very hacky but @edan-bainglass mentioned it is the only way it can work.

If I understand correctly, you want to have an entry point for the https:///uptime. I did this for template of the customize of jupyterhub index page in https://github.com/aiidalab/aiidalab-demo-server/blob/main/basehub/templates/hub-configmap.yaml
There is this config file you want to change https://github.com/aiidalab/aiidalab-demo-server/blob/main/basehub/files/etc/jupyterhub/jupyter_notebook_config.py.
Thus I think these changes need to moved there only for the demo server deployment.

A few things to clarify:

I mentioned it is the only way I could get it to work, as all other attempts failed. However, it is likely that I'm simply not following the correct approach, but the docs are lacking.

This feature is not only for the demo server deployment. It is a general feature we should offer to all "temporary" deployments

Indeed, one can adjust on the deployment side. One alternative to implementing this feature here would be to create a general deployment template and implement it there. This has been discussed, but not yet resolved.

That said, what is the issue with the feature living here? There is likely a cleaner solution to the echo solution.

This feature is not only for the demo server deployment.

That's true, this count down thing can be generic for other deployment.

That said, what is the issue with the feature living here?

I afraid even after this change when it deployment to the k8s, you still not see the timer there, since in our deployment, the jupyter_conifg.py is also override, I don't know which one will happened after and take effect. I think you did the correct change in this PR, but it requires more thoughts and test on the real k8s deployment and we decide where to put this changes.

So my calls are:

stack/lab/before-notebook.d/70_prepare_countdown_config.sh stay in this repo.

stack/lab/countdown/custom.cssstay in this repo.

stack/lab/countdown/custom.js stay in this repo.

stack/lab/aiidalab_uptime.py also stay here.

only the changes in the docker file become the deployment specific thing and goes to demo-server repo and take effect there. Plus you need to add some documentation on how to use this count down thing if we want to make it generic for future customize deployment. thoughts?

Agreed. I'll dig some more into how we introduce the endpoint in a cleaner way. And docs for sure. Thanks @unkcpz 🙏

Solved. See description.

unkcpz · 2025-04-04T08:24:49Z

@edan-bainglass my apologize that I am not able to give it a detail go in next two weeks. I am sick today and will start 2 weeks holiday next monday.

Since you notice the jupyter_notebook_config.py both in image and on the deployment, I think the changes should on the correct track.
There are further doc that worth to read through (I believe you already did):

edan-bainglass · 2025-04-04T08:38:43Z

@edan-bainglass my apologize that I am not able to give it a detail go in next two weeks. I am sick today and will start 2 weeks holiday next monday.

Again, sorry to hear about your illness 😔

Since you notice the jupyter_notebook_config.py both in image and on the deployment, I think the changes should on the correct track. There are further doc that worth to read through (I believe you already did):

https://jupyter-server.readthedocs.io/en/latest/users/configuration.html

https://jupyter-notebook.readthedocs.io/en/v6.4.6/config.html

Indeed, I went through these (and others) in the process of deriving my solution.

@danielhollas I'll give you some time to review/comment, but great if we can get this out soon. The demo server is now public on the AiiDAlab website. Great if we can have this feature integrated 🙏

danielhollas · 2025-04-07T04:23:11Z

I'll have a look today.

danielhollas · 2025-04-07T21:37:44Z

Hi @edan-bainglass,

this looks great, thanks for spending extra time on this. This is much closer what I imagined both in terms of UI and implementation. My feedback for the code is that I don't much like the custom lifetime parsing, but that can also be improved as a follow-up.

My main concern is similar to @unkcpz that I don't think this repo is the best place for this feature, at least for this first iteration.

That said, what is the issue with the feature living here? There is likely a cleaner solution to the echo solution.

The biggest downside is that this haven't been proven in production yet, and it's likely it will need tweaks or bugfixes. You'll be able to iterate much faster if all of this lives in the demo-stack repo or even the QeApp image.

While I agree that this can be a common feature, before we have an actual second use case, I'd prefer to not have it here. The docker stack here is already complex and quite painful to maintain, and this would add non-trivial complexity.

edan-bainglass · 2025-04-07T21:41:35Z

Fair. Will move to demo server repo until proven stable and/or required by other deployments. Thanks for the review @danielhollas and @unkcpz 🙏

danielhollas · 2025-04-04T14:39:32Z

stack/lab/countdown/custom.js

+require(["base/js/namespace", "base/js/events"], (Jupyter, events) => {
+  const parseLifetimeToMs = (str) => {
+    // Supports MM:SS, HH:MM:SS, or D-HH:MM:SS formats
+    const regex = /^(?:(\d+)-)?(?:(\d+):)?(\d+):(\d+)$/;


Not stoked about this regex and the custom parsing. Can we instead use some native API, perhaps the Date object? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date#date_time_string_format

These APIs tend to work best with standard time formats, of which the one I'm working with here is not. Regex gives me more precise control of the format, as well as a clean way to provide defaults. This is true in both languages.

danielhollas · 2025-04-04T14:43:40Z

stack/lab/Dockerfile

+COPY countdown/ ${CONDA_DIR}/lib/python${PYTHON_MINOR_VERSION}/site-packages/notebook/static/custom/
+
+# Add endpoint to fetch container uptime
+COPY aiidalab_uptime.py ${CONDA_DIR}/lib/python${PYTHON_MINOR_VERSION}/site-packages/


Ugh, directly copying a python script to site-packages is naughty 😅

It doesn't have to live there

danielhollas · 2025-04-04T14:46:19Z

stack/lab/aiidalab_uptime.py

+
+    def _parse_etime_to_seconds(self, etime):
+        # Supports MM:SS, HH:MM:SS, or D-HH:MM:SS formats
+        match = re.match(r"(?:(\d+)-)?(?:(\d+):)?(\d+):(\d+)", etime)


Can we use some native parsing from stdlib, perhaps datetime or time modules?

https://docs.python.org/3/library/datetime.html

Same comment as above

danielhollas · 2025-04-07T21:19:52Z

stack/lab/before-notebook.d/70_prepare_countdown_config.sh

+
+cat <<EOF >"${CUSTOM_DIR}/config.json"
+{
+  "ephemeral": $([ -n "$LIFETIME" ] && echo 1 || echo 0),


LIFETIME is quite generic. Perhaps CONTAINER_LIFETIME conveys its meaning better, if I understood the use case correctly?

Hmm, I think I'll keep this as is for this version. Happy to discuss this with the team at the next meeting.

stack/lab/countdown/custom.js

danielhollas · 2025-04-07T21:39:49Z

stack/lab/countdown/custom.js

+  };
+
+  const insertCountdown = (remainingMs) => {
+    if (document.getElementById("culling-countdown")) {


Does app_initialized.NotebookApp event ensure that this CSS will be loaded by the time this function gets called?

Hmm, don't I inject the element in the DOM before this is called?

Hmm, it looks like you create the culling-countdown banner element at the end of insertCountdown function? So is this if condition just guarding against it being inserted twice?

side note: Maybe using 4 spaces for indentation would help readability here, it's hard to keep up with all the nested functions / callbacks :D

Hmm, it looks like you create the culling-countdown banner element at the end of insertCountdown function? So is this if condition just guarding against it being inserted twice?

Right. Sorry, I didn't have the code in front of me when I replied.

side note: Maybe using 4 spaces for indentation would help readability here, it's hard to keep up with all the nested functions / callbacks :D

This was done very quickly. I'll have another look and clean it up where appropriate.

stack/lab/countdown/custom.js

This PR migrates aiidalab/aiidalab-docker-stack#525 to the demo server repo for production testing. Further details and discussion can be found in the original PR. One major difference is that endpoints for `config.json` and `uptime` are now referenced w.r.t `Jupyter.notebook.baseUrl`, which works in any environment.

Implement opt-in countdown feature and add to image recipe

7fd151e

edan-bainglass mentioned this pull request Apr 1, 2025

Implement opt-in countdown feature and add to image recipe #524

Closed

unkcpz requested changes Apr 2, 2025

View reviewed changes

edan-bainglass force-pushed the countdown branch 3 times, most recently from 4aab524 to b482dd2 Compare April 2, 2025 14:46

Fetch container uptime in timezone-independent way

926c25e

edan-bainglass force-pushed the countdown branch from 6e1ab16 to 926c25e Compare April 2, 2025 14:58

danielhollas self-requested a review April 2, 2025 15:42

edan-bainglass requested a review from unkcpz April 3, 2025 06:37

danielhollas requested changes Apr 7, 2025

View reviewed changes

edan-bainglass mentioned this pull request Apr 8, 2025

Implement countdown feature and include in deployment aiidalab/aiidalab-demo-server#39

Merged

danielhollas marked this pull request as draft April 15, 2025 12:36

Implement opt-in countdown feature and add to image recipe #525

Are you sure you want to change the base?

Implement opt-in countdown feature and add to image recipe #525

Uh oh!

Conversation

edan-bainglass commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Container uptime

To be considered

Uh oh!

edan-bainglass commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

unkcpz commented Apr 2, 2025

Uh oh!

unkcpz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

unkcpz commented Apr 4, 2025

Uh oh!

edan-bainglass commented Apr 4, 2025

Uh oh!

danielhollas commented Apr 7, 2025

Uh oh!

danielhollas commented Apr 7, 2025

Uh oh!

edan-bainglass commented Apr 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

edan-bainglass commented Apr 1, 2025 •

edited

Loading

edan-bainglass commented Apr 2, 2025 •

edited

Loading