- 
                Notifications
    
You must be signed in to change notification settings  - Fork 15
 
Implement opt-in countdown feature and add to image recipe #524
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
Conversation
821b574    to
    7fd151e      
    Compare
  
    | 
           Unclear as to why the test is failing. Seems unrelated, but surely I'm missing something.  | 
    
          
 Please reopen the PR from origin, otherwise you won't have permissions to push to ghcr.io  | 
    
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.
let's wait a bit to see if there are standard way to get the remaining cull time, I asked in https://discourse.jupyter.org/t/how-to-get-the-remaining-cull-time-for-the-running-container/34244
I also not so sure this timeout should put in the aiidalab-docker-stack. Is the timeout only for qeapp? If so, probably the better solution is to set a k8s timeout there instead of using the cull time from jupyter server.
To make the container closed at exactly after it start 12h during the deployment to k8s I believe we can use activeDeadlineSeconds https://kubernetes.io/docs/concepts/workloads/controllers/job/#job-termination-and-cleanup
With that approach there is not much need to change with this PR, the JS/css still fit and can be used for deployment when the timeout is set.
I didn't review the js part (I can read but not qualified to review, and I refuse to learn js 😎 ), css part looks good.
| # Set up opt-in countdown feature | ||
| 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/ | ||
| 
               | 
          
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.
These lines were moved to here when we debug it together. Can you move it to where the logo.png is copied, and see if it still work?
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.
It doesn't work due to the order of events I believe
          
 I'm not sure I understand your concerns. Let's discuss in person. 
 This is for any deployment of a temporary nature. General feature unrelated to any app. 
 The JS code is migrated mostly as is from aiidalab-timer and was already reviewed by @superstar54, who is of course welcome to give this another look 🙂  | 
    
| 
           Relocating to #525 due to permissions  | 
    
This PR reimplemented 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 AiiDAlabcustom.css- styling the countdown-related DOM as a sticky banner above the native navbarconfig.json.template- a template with an opt-in toggle and a lifetime variable, to be read bycustom.jsThe PR also introduces a new script under
before_notebook.dthat runs on container creation and replacesconfig.json.templatewithconfig.json, applying theEPHEMERALandEXPIRYenvironment variables, the latter computed from the opt-inLIFETIMEenvironment variable (used by each deployment), and the former set totrueifLIFETIMEis present,falseotherwise. This allows each deployment to opt-in and choose its own expiration.Lastly, the PR updates the
lab/Dockerfileto copy the necessary files tosite-packages/notebook/static/custom/and add thePYTHON_MINOR_VERSIONenvironment variable required by the container-runtime script.