Makefile: new target refresh-notebooks to refresh all notebook outputs #118
Conversation
Useful when server changed and notebook code did not change but we need to batch refresh all notebook outputs to match the new server for target test-notebooks to pass. Also add missing help for existing test-notebooks target. Add `nbconvert` to `requirements_dev.txt` for the new command `jupyter nbconvert` used. Had trouble installing `nbconvert` from Pip, had to use Conda, hope it'll be okay for other.
Because the notebooks are also used as tutorials, end users should see the result as if the notebooks are run against the real PAVICS deployment, not a localhost deployment.
…quent commands error in the future
We were allowing errors before so if the first notebook fail, we still proceed to try all the subsequent notebooks. But the danger with this allow errors is we might not notice the error and commit garbage!
|
@huard I have a habit to copy most of the relevant stuff in the PR description and in the comments, if any, into the merge commit itself. This is useful so all the important info is preserved in the physical git repo instead of on the PR. When no internet is available or PR is lost, we can still use the git history which is guarantied to always be available. If you merge my commits, can you do it next time? Thanks. |
|
👍 sorry ! |
| .PHONY: refresh-notebooks | ||
| refresh-notebooks: | ||
| @echo "Refresh all notebook outputs under docs/source/notebooks" | ||
| cd docs/source/notebooks; for nb in *.ipynb; do FINCH_WPS_URL="$(FINCH_WPS_URL)" jupyter nbconvert --to notebook --execute --ExecutePreprocessor.timeout=60 --output "$$nb" "$$nb"; sed -i "s@$(FINCH_WPS_URL)/outputs/@$(PAVICS_OUTPUT_URL)/@g" "$$nb"; done; cd $(APP_ROOT) |
There was a problem hiding this comment.
@huard FYI this code can literally be copy/paste as-is to other birds if we name FINCH_WPS_URL WPS_URL instead and use the same in all other birds. Something to keep in mind for future birds.
There was a problem hiding this comment.
Yes, my plan is to include it in the cookie-cutter, then merge it into the other birds.
@cehbrecht
There was a problem hiding this comment.
Since we are on the topic, cookie-cutter is great to start a new project but is not so great to keep the project up-to-date with the updated templates.
To avoid merge error when refreshing the template, I think we should at least find ways so files in the templates are re-useable as-is without any modifications.
I think we should think about making some kind of "makefile lib" with generic targets that gets customized by each project Makefile variable (ex: APP_NAME).
If we can prevent merge errors, then we can eventually think about automating the template refresh so refresh happen much more frequently.
There was a problem hiding this comment.
I've tried using cruft in a few of my projects and it works very well to track changes to boilerplate configurations but it's a nightmare to get it working if you don't know the exact cookiecutter template and version that you used to start with (it's worse if down the line you corrected a typo in the fields of your template; I can go deeper into this). One way around this would be to tag a cookiecutter template when we use it to create a service (e.g. git tag v0.0.1-pelican) so that we know which version we used to start with.
I think for our existing projects, @tlvu 's approach is reasonable. We should maintain and keep the cookiecutter template up to date as well with changes shared between them. There are a few makefile recipes that aren't portable (e.g. Raven) but perhaps we can extend the makefile for bird-specific recipes?
There was a problem hiding this comment.
Yes, solutions to painlessly port cookie-cutter template changes to birds would be very welcome. There is an open PR about this (bird-house/cookiecutter-birdhouse#79) but I consider it experimental. Other approaches / further testing would be appreciated.
Useful when server changed and notebook code did not change but we need
to batch refresh all notebook outputs to match the new server for target
test-notebooks to pass.
localhostoutput url is replace by PAVICS output url so notebooks is suitable astutorials against real PAVICS deployment.
Also add missing help for existing test-notebooks target.
Add
nbconverttorequirements_dev.txtfor the new commandjupyter nbconvertused. Had trouble installingnbconvertfrom Pip, had touse Conda, hope it'll be okay for other.
Overview
This PR fixes [issue id]
Changes:
Related Issue / Discussion
bird-house/cookiecutter-birdhouse#81
Additional Information
Links to other issues or sources.