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

Add notebooks to testing #24

Merged
merged 7 commits into from
Sep 18, 2020
Merged

Add notebooks to testing #24

merged 7 commits into from
Sep 18, 2020

Conversation

nikola-rados
Copy link
Contributor

Changes

  • Updates and moves original demo notebook
  • Updates Makefile commands
  • Mounts storage in docker-compose files

Resolves #20.

Notes

Originally, we wanted the notebooks to be included in the pytest suite. However, a recent update to the Makefile introduced some commands that would run the notebooks as a suite for you using nbval. All we had to do was hook in our notebook and we were good to go.

One slight complication was the password for the connection_string could not be directly used in the notebook. To get around this, the Makefile will create a credentials.json file which will store the secret. There is a dummy value setup by this process and the user must manually add the correct value. This has to be done before the notebooks will work correctly. The file is in the .gitignore so there should be no worry about exposing the passphrase publicly.

@nikola-rados nikola-rados added the enhancement New feature or request label Sep 17, 2020
@nikola-rados nikola-rados self-assigned this Sep 17, 2020
Copy link
Contributor

@cairosanders cairosanders left a comment

Choose a reason for hiding this comment

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

docs/source/notebooks/example.ipynb::Cell 0
  /tmp/sandpiper-venv/lib/python3.8/site-packages/jupyter_client/manager.py:66: DeprecationWarning: KernelManager._kernel_spec_manager_changed is deprecated in traitlets 4.1: use @observe and @unobserve instead.
    def _kernel_spec_manager_changed(self):

-- Docs: https://docs.pytest.org/en/stable/warnings.html

I just got this warning but otherwise all the tests passed!

Copy link
Contributor

@sum1lim sum1lim left a comment

Choose a reason for hiding this comment

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

LGTM! Demo and make command are working great.

@@ -5,3 +5,5 @@ venv/
*.sqlite
.ipynb_checkpoints/
.Rhistory
credentials.json
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! It's nice that you thought of this.

@@ -132,7 +138,7 @@ notebook-sanitizer:
.PHONY: test-notebooks
test-notebooks: notebook-sanitizer
@echo "Running notebook-based tests"
@bash -c "env WPS_URL=$(WPS_URL) pytest --nbval --verbose $(CURDIR)/docs/source/notebooks/ --sanitize-with $(CURDIR)/docs/source/output-sanitize.cfg --ignore $(CURDIR)/docs/source/notebooks/.ipynb_checkpoints"
@bash -c "source $(VENV)/bin/activate && env WPS_URL=$(WPS_URL) pytest --nbval --verbose $(CURDIR)/docs/source/notebooks/ --sanitize-with $(CURDIR)/docs/source/output-sanitize.cfg --ignore $(CURDIR)/docs/source/notebooks/.ipynb_checkpoints"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the commands are being executed when running make command. Also, I don't understand what the commands do. Could you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The command is mostly setup. Setting up the environment and sanitizing the notebook before running pytest with nbval to run the notebook.

Makefile Show resolved Hide resolved
@@ -5,7 +5,8 @@ services:
image: pcic/sandpiper
ports:
- "8101:5003"

volumes:
- /storage/data/climate/downscale/BCCAQ2/:/storage/data/climate/downscale/BCCAQ2/
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder what this line is for. Would be appreciated if you could teach me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I discovered that our deployed instance of sandpiper was unable to correctly resolve the rules as it did not have access to the data. This just supplies the data as a volume that can be targeted using the same directory structure the database anticipates.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notebooks in pytest
3 participants