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

provide details for debugging failing test-thredds notebook #87

Merged
merged 3 commits into from
Aug 24, 2021

Conversation

fmigneault
Copy link
Contributor

Overview

Adds general debugging utilities regarding notebook test_thredds

Changes

  • Adds variable overrides in test_thredds for local debugging.
  • Adds important documentation notes regarding possible error debugging leads for test_thredds (based on actual issues encountered).

Related Issue / Discussion

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@fmigneault fmigneault requested a review from tlvu August 10, 2021 22:57
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.

LGTM. As usual, copy/paste PR description into merge commit description. Thx.

@fmigneault fmigneault merged commit 10a784a into master Aug 24, 2021
@fmigneault fmigneault deleted the test-thredds-meta branch August 24, 2021 19:39
@@ -56,15 +56,15 @@
"import requests\n",
"print(\"Setup configuration parameters...\")\n",
"\n",
"PAVICS_HOST = os.getenv(\"PAVICS_HOST\", \"pavics.ouranos.ca\").rstrip(\"/\")\n",
"PAVICS_HOST = (os.getenv(\"PAVICS_HOST\") or \"pavics.ouranos.ca\").rstrip(\"/\")\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, os.getenv("VAR_NAME", default_value) do not work so you have to do os.getenv("VAR_NAME") or default_value? I know both achieve the same effect so I am not against the other format but I just wonder what is the subtleties that make you change to the other form.

Copy link
Contributor Author

@fmigneault fmigneault Aug 24, 2021

Choose a reason for hiding this comment

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

os.getenv("VAR_NAME", default_value) will return an empty string or None if the variable is defined with VAR_NAME= or VAR_NAME="". In this case, I want to consider empty equivalent as undefined and revert to the default. The default_value is picked only if there was no export VAR_NAME anywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see! That "empty value" problem did happen via Jenkins build request or you are being defensive for when invoking pytest manually directly on the command-line and user can accidentally introduce the problem?

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 had this issue when testing locally.
I have a script that allows me to quickly source an .env file and spin up a Jupyter server connected to a deployed daccs-iac instance. The issue I faced was that this script only updates variables (cannot unset them), so I was overriding one of them as export VAR= to "unset" it, but the empty string remained.

I don't think it would impact any automated test suites because they all set their variables once.

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.

None yet

2 participants