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

recompute OWSRegistry on demand to avoid caching issues with db-session #111

Merged
merged 2 commits into from
Dec 1, 2021

Conversation

fmigneault
Copy link
Contributor

Description

This resolves many long standing issues related to request caching negotiation between Magpie and Twitcher.
As illustrated in this test results, many requests are failing due to NoTransaction error:
bird-house/birdhouse-deploy#197 (comment)

Following this fix (manually integrated in Ouranosinc/Magpie@ae1698a (#488) and corresponding Magpie & Twitcher Dockers pushed with manual tags), following results are obtained.

ignore weaver errors that are unrelated, all else are gone including notably stress-tests
http://daccs-jenkins.crim.ca/job/DACCS-iac-birdhouse/789
http://daccs-jenkins.crim.ca/job/PAVICS-e2e-workflow-tests/job/master/672

14:33:53  ============================= test session starts ==============================
14:33:53  platform linux -- Python 3.7.10, pytest-6.2.5, py-1.10.0, pluggy-0.13.1
14:33:53  rootdir: /home/jenkins/agent/workspace/PAVICS-e2e-workflow-tests_master
14:33:53  plugins: anyio-3.3.0, dash-1.21.0, nbval-0.9.6, tornasync-0.6.0.post2
14:33:53  collected 222 items
14:33:53  
14:34:07  notebooks-auth/test_thredds.ipynb ...........                            [  4%]
14:34:33  pavics-sdi-master/docs/source/notebooks/WCS_example.ipynb .......        [  8%]
14:34:43  pavics-sdi-master/docs/source/notebooks/WFS_example.ipynb ......         [ 10%]
14:34:52  pavics-sdi-master/docs/source/notebooks/WMS_example.ipynb ........       [ 14%]
14:34:55  pavics-sdi-master/docs/source/notebooks/WPS_example.ipynb ..........     [ 18%]
14:35:10  pavics-sdi-master/docs/source/notebooks/esgf-dap.ipynb .                 [ 19%]
14:35:11  pavics-sdi-master/docs/source/notebooks/jupyter_extensions.ipynb .       [ 19%]
14:35:16  pavics-sdi-master/docs/source/notebooks/opendap.ipynb .......            [ 22%]
14:35:23  pavics-sdi-master/docs/source/notebooks/pavics_thredds.ipynb .....       [ 25%]
14:39:13  pavics-sdi-master/docs/source/notebooks/regridding.ipynb ............... [ 31%]
14:40:04  ..............                                                           [ 38%]
14:40:11  pavics-sdi-master/docs/source/notebooks/rendering.ipynb ....             [ 40%]
14:40:13  pavics-sdi-master/docs/source/notebooks/subset-user-input.ipynb ........ [ 43%]
14:40:42  .................                                                        [ 51%]
14:40:49  pavics-sdi-master/docs/source/notebooks/subsetting.ipynb .....           [ 53%]
14:40:52  pavics-sdi-master/docs/source/notebook-components/weaver_example.ipynb . [ 54%]
14:40:54  .FFFFFFFF.                                                               [ 58%]
14:41:06  finch-master/docs/source/notebooks/dap_subset.ipynb ..........           [ 63%]
14:41:15  finch-master/docs/source/notebooks/finch-usage.ipynb ......              [ 65%]
14:41:59  finch-master/docs/source/notebooks/subset.ipynb ....................     [ 74%]
14:42:00  PAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-1DataAccess.ipynb . [ 75%]
14:42:05  ......                                                                   [ 77%]
14:42:44  PAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-2Subsetting.ipynb . [ 78%]
14:43:04  .............                                                            [ 84%]
14:43:15  PAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-3Climate-Indicators.ipynb . [ 84%]
14:45:47  ....s.                                                                   [ 87%]
14:45:59  PAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-4Ensembles.ipynb . [ 87%]
14:46:06  ...                                                                      [ 89%]
14:46:33  PAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-5Visualization.ipynb . [ 89%]
14:47:01  ......                                                                   [ 92%]
14:47:06  notebooks/hummingbird.ipynb ............                                 [ 97%]
14:50:32  notebooks/stress-tests.ipynb .....                                       [100%]
14:50:32  
14:50:32  =================================== FAILURES ===================================

After this PR is integrated, I will properly tag/bump Twitcher with 0.6.2 and apply it to Magpie for 3.19.0 release.

References

@fmigneault
Copy link
Contributor Author

@dbyrns Couldn't tag you as reviewer, just FYI.
@ChaamC This should unblock your PR very soon. You will need to update to magpie 3.19.0 once everything is released.

@codecov-commenter
Copy link

codecov-commenter commented Nov 30, 2021

Codecov Report

Merging #111 (4fecbe2) into master (9b6426d) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #111   +/-   ##
=======================================
  Coverage   72.16%   72.16%           
=======================================
  Files          40       40           
  Lines        1703     1703           
=======================================
  Hits         1229     1229           
  Misses        474      474           
Impacted Files Coverage Δ
twitcher/owsregistry.py 58.33% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b6426d...4fecbe2. Read the comment docs.

Copy link

@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.

LGTM

@cehbrecht
Copy link
Member

@fmigneault feel free to merge.

@fmigneault fmigneault merged commit 09c4b6c into master Dec 1, 2021
@fmigneault fmigneault deleted the fix-caching-owsregistry branch December 1, 2021 15:46
@fmigneault
Copy link
Contributor Author

@cehbrecht
Packages v0.6.1 and v0.6.2 are not released on PyPI.
Is there a manual step to deploy them?

@cehbrecht
Copy link
Member

@fmigneault I have uploaded the packages now. I have invited you as maintainer on pypi.

@fmigneault
Copy link
Contributor Author

@cehbrecht
Do you think it would be possible to add a bot or add a secret token to let the GitHub CI do it automatically when a tag is detected?

@cehbrecht
Copy link
Member

Do you think it would be possible to add a bot or add a secret token to let the GitHub CI do it automatically when a tag is detected?

So far, I have done the upload always manually. @Zeitsperre do you have experience with this? Would you recommend to have it in the CI?

@fmigneault
Copy link
Contributor Author

@cehbrecht
I believe you must be doing a twine upload dist/* call to update the package?
I so, I believe having the CI that already triggers the docker image build would be the right way to update packages.
I also just noticed, but Github releases are also out of sync, and could be deployed by the same CI step when a tag is detected.

@cehbrecht
Copy link
Member

@fmigneault yes ... using twine to upload the package. I have updated now the GitHub releases.

@Zeitsperre do you have experience with release automation in CI? I think I have seen something in a ci config ... but it was not used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants