Skip to content

Override notebooks within Jenkinsfile#152

Merged
Zeitsperre merged 10 commits intomasterfrom
config-override-notebooks
Nov 11, 2025
Merged

Override notebooks within Jenkinsfile#152
Zeitsperre merged 10 commits intomasterfrom
config-override-notebooks

Conversation

@Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Aug 22, 2025

Overview

This PR adds a new filtering mechanism to make it easier to quickly run tests against individual notebooks.

Changes

  • Modifies the Jenkinsfile to accept a new ALLOWLIST_NOTEBOOKS field for providing a filter for notebooks to be run

Testing Checklist

  • Manually tested and verified working on Jenkins

Related Issue / Discussion

The functionality was already possible by setting a gist with a filtering function and setting CONFIG_PARAMETERS_SCRIPT_URL to pick up the script, overriding existing settings.

The changes here make it so that this can simply be performed directly on Jenkins.

@Zeitsperre
Copy link
Collaborator Author

@tlvu

I get the feeling you know where I'm going with this PR. Can you explain to me how I can pass environment variables from the Jenkins UI so that they're only expanded in the testall shell script?

Copy link
Contributor

@tlvu tlvu left a comment

Choose a reason for hiding this comment

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

Minor suggestions. I'll need to test this myself once you are done. But this looks good to me. Let me know what's not working?

@tlvu tlvu self-requested a review August 22, 2025 19:14
@tlvu
Copy link
Contributor

tlvu commented Aug 22, 2025

Can you explain to me how I can pass environment variables from the Jenkins UI so that they're only expanded in the testall shell script?

I think in the current state things should work. What's not working?

@tlvu
Copy link
Contributor

tlvu commented Aug 22, 2025

Please remove the boilerplate in the PR description. That boilerplate was for a new Jupyter env docker image release. It's not for this case. Please summarize your change short and sweet in the PR description. No need for that boilerplate.

@Zeitsperre
Copy link
Collaborator Author

I think in the current state things should work. What's not working?

Specifically, the environment variables from the Jenkins UI are not being expanded in the testall shell script:

http://jenkins.ouranos.ca/job/PAVICS-e2e-workflow-tests/job/config-override-notebooks/9/console

For pytest:

10:56:26  + py.test --rootdir=. --nbval $PAVICS_SDI_DIR/docs/source/notebooks/subset-user-input.ipynb $PAVICS_SDI_DIR/docs/source/notebooks/subsetting.ipynb --nbval-sanitize-with notebooks/output-sanitize.cfg --dist=loadscope --numprocesses=0
10:56:29  ERROR: file or directory not found: $PAVICS_SDI_DIR/docs/source/notebooks/subset-user-input.ipynb
10:56:29  
10:56:29  ============================= test session starts ==============================
10:56:29  platform linux -- Python 3.11.12, pytest-8.3.5, pluggy-1.5.0
10:56:29  rootdir: /home/jenkins/agent/workspace/_tests_config-override-notebooks@2
10:56:29  plugins: anyio-4.9.0, dash-3.0.3, nbval-0.11.0, tornasync-0.6.0.post2, xdist-3.6.1
10:56:29  collected 0 items
10:56:29  
10:56:29  ============================ no tests ran in 0.01s =============================
10:56:29  + EXIT_CODE=4

for nbconvert:

10:56:31  + enable_resulting_nb $PAVICS_SDI_DIR/docs/source/notebooks/subsetting.ipynb
10:56:31  + [ x$PAVICS_SDI_DIR/docs/source/notebooks/subsetting.ipynb = xpavics-sdi-master/docs/source/notebooks/FAQ_dask_parallel.ipynb ]
10:56:31  + [ x$PAVICS_SDI_DIR/docs/source/notebooks/subsetting.ipynb = xPAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-3Climate-Indicators.ipynb ]
10:56:31  + return 0
10:56:31  + jupyter nbconvert --to notebook --execute --ExecutePreprocessor.timeout=240 --allow-errors --output-dir buildout --output $PAVICS_SDI_DIR/docs/source/notebooks/subsetting.output.ipynb $PAVICS_SDI_DIR/docs/source/notebooks/subsetting.ipynb
10:56:32  [NbConvertApp] WARNING | pattern '$PAVICS_SDI_DIR/docs/source/notebooks/subsetting.ipynb' matched no files
10:56:32  This application is used to convert notebook files (*.ipynb)
10:56:32          to various other formats.
...

@Zeitsperre
Copy link
Collaborator Author

Please remove the boilerplate in the PR description. That boilerplate was for a new Jupyter env docker image release. It's not for this case. Please summarize your change short and sweet in the PR description. No need for that boilerplate.

I'm going to propose some new templates for Pull Requests.

@tlvu
Copy link
Contributor

tlvu commented Aug 26, 2025

Specifically, the environment variables from the Jenkins UI are not being expanded in the testall shell script

Ah ! True, you'll need to eval that new CUSTOM_NOTEBOOKS var before you can use it.

@tlvu
Copy link
Contributor

tlvu commented Nov 10, 2025

@Zeitsperre Going through my old PRs to review, I've replied to you for this one. Have you tried it? Why is this PR in Draft mode by the way? Not sure you want to productize it?

@Zeitsperre Zeitsperre marked this pull request as ready for review November 11, 2025 19:37
Copy link
Contributor

@tlvu tlvu left a comment

Choose a reason for hiding this comment

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

Please just fix the PR description before merge.

@Zeitsperre Zeitsperre merged commit c54fb67 into master Nov 11, 2025
1 check failed
@Zeitsperre Zeitsperre deleted the config-override-notebooks branch November 11, 2025 21:44
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.

2 participants