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.

Sign up for GitHub

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

Jump to bottom

fix: convert Path to str. update notebook #114

Merged
merged 5 commits into from
Mar 18, 2020
Merged

fix: convert Path to str. update notebook #114

merged 5 commits into from
Mar 18, 2020

Conversation

huard
Copy link
Collaborator

@huard huard commented Mar 17, 2020

Overview

This PR fixes #113

Changes:

  • Fixed wps_xclim_indices. Possibly same issue elsewhere, but wondering why it didn't pop up before.
  • Updated notebook. finch notebook in pavics-sdi can be deleted.

@huard huard requested review from davidcaron and tlvu March 17, 2020 16:46
tlvu
tlvu reviewed Mar 17, 2020
View reviewed changes
docs/source/notebooks/finch-usage.ipynb Outdated
" output_netcdf='https://pavics.ouranos.ca/wpsoutputs/e8afbb04-42d3-11ea-9531-0242ac12000b/frost-days_SRES-A2-experiment_20460101-20650101.nc',\n",
" output_log='https://pavics.ouranos.ca/wpsoutputs/e8afbb04-42d3-11ea-9531-0242ac12000b/log.txt',\n",
" ref='https://pavics.ouranos.ca/wpsoutputs/e8afbb04-42d3-11ea-9531-0242ac12000b/input.meta4'\n",
" output_netcdf='http://localhost:5000/outputs/13b909e4-686e-11ea-a94b-b052162515fb/frost-days_SRES-A2-experiment_20460101-20650101.nc',\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert all output url from localhost to pavics.ouranos.ca? With the new regex, leaving it as localhost will pass Jenkins but given this notebooks is used as tutorial, it's weird the output url do not match the host.

Your call, for me what's most important is Jenkins is not broken.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But how do I update the notebook before the working server is live at pavics.ouranos.ca ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

But how do I update the notebook before the working server is live at pavics.ouranos.ca ?

Manual, just for that cell ;( Yeah it's ugly. You can leave it as localhost as well, won't break Jenkins. Users running the tutorials notebook might not notice as well.

docs/source/notebooks/finch-usage.ipynb Outdated
"output_type": "stream",
"text": [
"Metalink content-type detected.\n",
"Downloading to /tmp/tmp8fi43lm1/frost-days_SRES-A2-experiment_20460101-20650101.nc.\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This random path most likely will break Jenkins. But I don't have an updated Finch server to test. Will do a blind output-sanitize.cfg fix.

@tlvu
Copy link
Collaborator

tlvu commented Mar 17, 2020

Travis-CI error that do not look like regex related. Did you "Kernel -> Restart kernel and run all cells"?

_______________ docs/source/notebooks/finch-usage.ipynb::Cell 4 ________________

Notebook cell execution failed

Cell 4: Cell outputs differ

Input:

ds, log, metalink = resp.get(asobj=True)

Traceback:

Missing output fields from running code: {'stdout'}

_______________ docs/source/notebooks/finch-usage.ipynb::Cell 5 ________________

Notebook cell execution failed

Cell 5: Cell outputs differ

Input:

ds

Traceback:

Unexpected output fields from running code: {'text/html'}

tlvu added a commit to Ouranosinc/pavics-sdi that referenced this pull request Mar 17, 2020
@tlvu
finch.ipynb: deleted since duplicate with the one in Finch repo
6efc020 
Pr bird-house/finch#114 merged the explanation
differences between the 2 duplicate notebooks.
@tlvu tlvu mentioned this pull request Mar 17, 2020
Merged
tlvu added a commit to Ouranosinc/PAVICS-e2e-workflow-tests that referenced this pull request Mar 17, 2020
@tlvu
output-sanitize.cfg: add temp folder from Finch notebook
6cdcba3 
Blind fix for Pr bird-house/finch#114 since no
updated Finch server for testing.
@huard
Copy link
Collaborator Author

huard commented Mar 17, 2020

I thought I did... apparently not.

tlvu and others added 2 commits March 17, 2020 14:39
@tlvu
finch-usage.ipynb: make code backward compat with old Finch server
1608df8 
This is so we can release new notebook before releasing new server.

Note the output is not backward compatible, only the code is.

So Jenkins will still be broken, but at least the notebook so not appear
broken to user when they run it from the tutorial-notebooks folder on
the Jupyter env.

See discussion in comment
#112 (comment)
@huard
run all cells
7789be2
@tlvu
Copy link
Collaborator

tlvu commented Mar 17, 2020

@huard I just push new change to make the notebook backward compatible with old server, you'll have to pull and re-run all cells again.

@tlvu
Copy link
Collaborator

tlvu commented Mar 17, 2020

I thought I did... apparently not.

Oh it might be that your jupyterlab and notebook (update both just in case) is old and the output is recorded differently from the latest version from Travis-CI which cause the diff.

tlvu added a commit to Ouranosinc/pavics-sdi that referenced this pull request Mar 17, 2020
@tlvu
Merge pull request #157 from Ouranosinc/delete-duplicated-finch-notebook
8b07105 
finch.ipynb: deleted since duplicate with the one in Finch repo

Pr bird-house/finch#114 merged the explanation
differences between the 2 duplicate notebooks.

Fixes #156
@tlvu tlvu mentioned this pull request Mar 17, 2020
Merged
tlvu added a commit to Ouranosinc/PAVICS-e2e-workflow-tests that referenced this pull request Mar 17, 2020
@tlvu
Merge pull request #39 from Ouranosinc/output-sanitize-for-Finch-nb
ae73105 
output-sanitize.cfg: add temp folder from Finch notebook

Blind fix for Pr bird-house/finch#114 since no
updated Finch server for testing.

New regex not adding more errors to Jenkins vs old Finch: http://jenkins.ouranos.ca/job/PAVICS-e2e-workflow-tests/job/output-sanitize-for-Finch-nb/2/console
@huard huard merged commit 8b99e4c into master Mar 18, 2020
@huard huard deleted the fix-113 branch March 18, 2020 10:18
tlvu added a commit to bird-house/birdhouse-deploy that referenced this pull request Mar 19, 2020
@tlvu
finch: update to 0.5.1
a11200c 
To get fix for bird-house/finch#114 and
bird-house/finch#117
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tlvu tlvu tlvu approved these changes

@davidcaron davidcaron Awaiting requested review from davidcaron

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PyWPS does not accept Path objects
2 participants
@huard @tlvu