Jupyter: Restore terminal and allow recursive folder deletion #280
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It was disabled to avoid malicious usage but with the monitoring in place and the demo account restricted to limited resources, it's probably safe to try enabling this again. For legitimate users, not having the terminal is pretty annoying. Should not penalize legit users for some rogue users.
This was not possible before since non-empty dir deletion was not possible.
The container died instantly on creation startup.
Fix this error found during autodeploy:
```
fix GeoServer data dir permission on first run only, when data dir do not exist yet.
+ DATA_DIR='${DATA_PERSIST_ROOT}/geoserver'
+ '[' -n ]
+ docker run --rm --name fix-geoserver-data-dir-perm --volume '${DATA_PERSIST_ROOT}/geoserver:/datadir' --env FIRST_RUN_ONLY bash:5.1.4 bash -xc 'if [ -z "$FIRST_RUN_ONLY" -o ! -f /datadir/global.xml ]; \
then chown -R 1000:10001 /datadir; else echo "No execute."; fi'
docker: Error response from daemon: create ${DATA_PERSIST_ROOT}/geoserver: "${DATA_PERSIST_ROOT}/geoserver" includes invalid characters for a local volume name, only "[a-zA-Z0-9][a-zA-Z0-9_.-]" are allowed. If you intended to pass a host directory, use absolute path.
See 'docker run --help'.
```
…lways read last env.local will always have the latest updated values so no need for delayed eval.
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac/35/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/925/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1140/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/925/NOTEBOOK TEST RESULTS |
fmigneault
reviewed
Jan 25, 2023
There was a problem hiding this comment.
The changes look OK, but I wonder if it would not be better to have a separate .sh that does the full process_delayed_eval definition + . env.local + . default.env steps in the appropriate order. In other words, move the logic of sourcing ENV definitions together instead of duplicated in each script. It will be easier to maintain in case other approaches like in #272 (comment) are attempted.
Agreed. I think a further "logic grouping" is required. Retrospectively, maybe I should not have supported delayed eval in the first place. It's a bit more complex than initially planned for not a very big gain (only 3 variables so far). So I am not sure I want to put more effort into this feature. Let's keep it as-is for now and see how it evolves. But you are right, next time we leverage this feature again and decide to keep this feature, the "logic grouping" should be done at the same time. |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1142/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/927/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac/37/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/927/NOTEBOOK TEST RESULTS |
I think it would be better to group them right away, but without modifying the DELAYED_VARS approach yet. If this new method is applied right away and spreads across scripts (including user-specific overrides), it will have an even bigger impact with more restrictions to modify it later. |
Then I'll remove the fix in this PR and open a different PR to avoid delaying this PR. |
This reverts commit 6687185. To be implemented better in a different PR. Francis proposed to centralize all the sourcing logic of default.env and env.local and calling process_delayed_eval in a same file so all scripts to not have to remember the loading sequence and we have a central location to update that loading sequence without having to edit all the scripts in the future.
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1143/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/928/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac/38/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/929/NOTEBOOK TEST RESULTS |
huard
approved these changes
Jan 25, 2023
fmigneault
approved these changes
Jan 25, 2023
|
Looking at the nb failures.
|
I think it's because it looks for the Must be a precommit autoformat that breaks that line on multple lines. But I don't understand, I thought we escaped all the lines with
Seems to be test env configuration problem. User |
tlvu
changed the title
github-actions
bot
added
ci/operations
tlvu
deleted the
restore-jupyter-terminal-and-allow-recursive-folder-deletion
branch
|
Merged this PR anyways since the 2 failed notebooks are absolutely unrelated to my JupyterHub config change. |
The escape for the precommit autoformater to avoid wrapping the line was already done in this commit: Ouranosinc/pavics-sdi@75652fb Now sure how this escape got lost. I'll just restore it. |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1144/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL :NOTEBOOK TEST RESULTS
|
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac/39/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL :NOTEBOOK TEST RESULTS
|
huard
added a commit
to Ouranosinc/pavics-sdi
that referenced
this pull request
Jan 26, 2023
…277) Should have used the climex link on PAVICS prod server instead of the test server. `TEST_USE_PROD_DATA` should be on the same line as "pavics.ouranos.ca" to avoid it being replaced by the test server during Jenkins run. The precommit autoformatter should not break that line. See previous fix that was lost, not sure why 75652fb (PR #267) Jenkins failure found in PR bird-house/birdhouse-deploy#280 (comment): ``` 10:36:34 _________ pavics-sdi-master/docs/source/notebooks/climex.ipynb::Cell 0 _________ 10:36:34 Notebook cell execution failed 10:36:34 Cell 0: Cell execution caused an exception 10:36:34 10:36:34 Input: 10:36:34 import shutil 10:36:34 10:36:34 import intake 10:36:34 import xarray as xr 10:36:34 import xclim 10:36:34 from clisops.core.subset import subset_gridpoint 10:36:34 from dask.diagnostics import ProgressBar 10:36:34 from dask.distributed import Client, LocalCluster 10:36:34 from IPython.display import HTML, Markdown 10:36:34 from xclim import ensembles as xens 10:36:34 10:36:34 cat = intake.open_esm_datastore( 10:36:34 "https://host-140-154.rdext.crim.ca/catalog/climex.json" 10:36:34 ) # TEST_USE_PROD_DATA 10:36:34 cat.df.head() 10:36:34 10:36:34 Traceback: 10:36:34 10:36:34 --------------------------------------------------------------------------- 10:36:34 JSONDecodeError Traceback (most recent call last) 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/site-packages/requests/models.py:971, in Response.json(self, **kwargs) 10:36:34 970 try: 10:36:34 --> 971 return complexjson.loads(self.text, **kwargs) 10:36:34 972 except JSONDecodeError as e: 10:36:34 973 # Catch JSON-related errors and raise as requests.JSONDecodeError 10:36:34 974 # This aliases json.JSONDecodeError and simplejson.JSONDecodeError 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/json/__init__.py:357, in loads(s, cls, object_hook, parse_float, parse_int, parse_constant, object_pairs_hook, **kw) 10:36:34 354 if (cls is None and object_hook is None and 10:36:34 355 parse_int is None and parse_float is None and 10:36:34 356 parse_constant is None and object_pairs_hook is None and not kw): 10:36:34 --> 357 return _default_decoder.decode(s) 10:36:34 358 if cls is None: 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/json/decoder.py:337, in JSONDecoder.decode(self, s, _w) 10:36:34 333 """Return the Python representation of ``s`` (a ``str`` instance 10:36:34 334 containing a JSON document). 10:36:34 335 10:36:34 336 """ 10:36:34 --> 337 obj, end = self.raw_decode(s, idx=_w(s, 0).end()) 10:36:34 338 end = _w(s, end).end() 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/json/decoder.py:355, in JSONDecoder.raw_decode(self, s, idx) 10:36:34 354 except StopIteration as err: 10:36:34 --> 355 raise JSONDecodeError("Expecting value", s, err.value) from None 10:36:34 356 return obj, end 10:36:34 10:36:34 JSONDecodeError: Expecting value: line 4 column 1 (char 3) 10:36:34 10:36:34 During handling of the above exception, another exception occurred: 10:36:34 10:36:34 JSONDecodeError Traceback (most recent call last) 10:36:34 Cell In [1], line 12 10:36:34 9 from IPython.display import HTML, Markdown 10:36:34 10 from xclim import ensembles as xens 10:36:34 ---> 12 cat = intake.open_esm_datastore( 10:36:34 13 "https://host-140-154.rdext.crim.ca/catalog/climex.json" 10:36:34 14 ) # TEST_USE_PROD_DATA 10:36:34 15 cat.df.head() 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/site-packages/intake_esm/core.py:83, in esm_datastore.__init__(self, esmcol_obj, esmcol_data, progressbar, sep, csv_kwargs, **kwargs) 10:36:34 81 super(esm_datastore, self).__init__(**kwargs) 10:36:34 82 if isinstance(esmcol_obj, (str, pathlib.PurePath)): 10:36:34 ---> 83 self.esmcol_data, self.esmcol_path = _fetch_and_parse_json(esmcol_obj) 10:36:34 84 self._df, self.catalog_file = _fetch_catalog(self.esmcol_data, esmcol_obj, csv_kwargs) 10:36:34 86 elif isinstance(esmcol_obj, pd.DataFrame): 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/site-packages/intake_esm/utils.py:59, in _fetch_and_parse_json(input_path) 10:36:34 56 data = json.load(filein) 10:36:34 58 except Exception as exc: 10:36:34 ---> 59 raise exc 10:36:34 61 return data, input_path 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/site-packages/intake_esm/utils.py:52, in _fetch_and_parse_json(input_path) 10:36:34 50 if _is_valid_url(input_path): 10:36:34 51 resp = requests.get(input_path) 10:36:34 ---> 52 data = resp.json() 10:36:34 53 else: 10:36:34 54 input_path = Path(input_path).absolute().as_posix() 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/site-packages/requests/models.py:975, in Response.json(self, **kwargs) 10:36:34 971 return complexjson.loads(self.text, **kwargs) 10:36:34 972 except JSONDecodeError as e: 10:36:34 973 # Catch JSON-related errors and raise as requests.JSONDecodeError 10:36:34 974 # This aliases json.JSONDecodeError and simplejson.JSONDecodeError 10:36:34 --> 975 raise RequestsJSONDecodeError(e.msg, e.doc, e.pos) 10:36:34 10:36:34 JSONDecodeError: Expecting value: line 4 column 1 (char 3) ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes:
Jupyter: allow recursive directory deletion
This was not possible before since non-empty dir deletion was not possible.
Jupyter: re-enable terminal for all users
It was disabled to avoid malicious usage but with the monitoring in
place and the demo account restricted to limited resources, it's
probably safe to try enabling this again.
For legitimate users, not having the terminal is pretty annoying.
Should not penalize legit users for some rogue users.
Deployed to https://lvupavicsmaster.ouranos.ca for testing.