From 9439aa7972efc52730bd9071752981fc95a01c18 Mon Sep 17 00:00:00 2001 From: Ondrej Grover Date: Tue, 13 Jun 2023 09:20:35 +0200 Subject: [PATCH 1/6] check if migrated marker is actually readable better than just checking if it exists, because some on some platforms (e.g. ro AFS on HPC ) os.path.exists() may give False just because os.stat fails, even though the file actually exists Also adds more test coveraate of the migrated marker checking Fixes #355 --- jupyter_core/application.py | 14 +++++++++++--- jupyter_core/tests/test_migrate.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/jupyter_core/application.py b/jupyter_core/application.py index e572888..0402ec5 100644 --- a/jupyter_core/application.py +++ b/jupyter_core/application.py @@ -168,9 +168,17 @@ def ask(): def migrate_config(self): """Migrate config/data from IPython 3""" - if os.path.exists(os.path.join(self.config_dir, "migrated")): - # already migrated - return + try:# let's see if we can open the marker file + # for reading and updating (writing) + f_marker = open(os.path.join(self.config_dir, "migrated"), 'r+') + except PermissionError: # not readable and/or writable + return # so let' give up migration in such an environment + except FileNotFoundError: # cannot find the marker file + pass # that means we have not migrated yet, so continue + else: # if we got here without raising anything, + # that means the file exists + f_marker.close() + return # so we must have already migrated -> bail out from .migrate import get_ipython_dir, migrate diff --git a/jupyter_core/tests/test_migrate.py b/jupyter_core/tests/test_migrate.py index 776b3cc..9deef89 100644 --- a/jupyter_core/tests/test_migrate.py +++ b/jupyter_core/tests/test_migrate.py @@ -21,6 +21,8 @@ ) from jupyter_core.utils import ensure_dir_exists +from jupyter_core.application import JupyterApp + pjoin = os.path.join here = os.path.dirname(__file__) dotipython = pjoin(here, "dotipython") @@ -216,3 +218,30 @@ def test_migrate(env): migrate() assert os.path.exists(env["JUPYTER_CONFIG_DIR"]) assert os.path.exists(env["JUPYTER_DATA_DIR"]) + +def test_app_migrate(env): + shutil.copytree(dotipython, env["IPYTHONDIR"]) + app = JupyterApp() + app.initialize([]) + assert os.path.exists(env["JUPYTER_CONFIG_DIR"]) + assert os.path.exists(env["JUPYTER_DATA_DIR"]) + + +def test_app_migrate_skip_if_marker(env): + shutil.copytree(dotipython, env["IPYTHONDIR"]) + touch(pjoin(env["JUPYTER_CONFIG_DIR"], "migrated"), "done") + app = JupyterApp() + app.initialize([]) + assert os.listdir(env["JUPYTER_CONFIG_DIR"]) == ["migrated"] + assert not os.path.exists(env["JUPYTER_DATA_DIR"]) + + +def test_app_migrate_skip_unwritable_marker(env): + shutil.copytree(dotipython, env["IPYTHONDIR"]) + migrated_marker = pjoin(env["JUPYTER_CONFIG_DIR"], "migrated") + touch(migrated_marker, "done") + os.chmod(migrated_marker, 0) # make it unworkable + app = JupyterApp() + app.initialize([]) + assert os.listdir(env["JUPYTER_CONFIG_DIR"]) == ["migrated"] + assert not os.path.exists(env["JUPYTER_DATA_DIR"]) From eca1e8d3e9d00b1ffe38cd47b45f1864cb029201 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 13 Jun 2023 07:26:23 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- jupyter_core/application.py | 10 +++++----- jupyter_core/tests/test_migrate.py | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/jupyter_core/application.py b/jupyter_core/application.py index 0402ec5..b100764 100644 --- a/jupyter_core/application.py +++ b/jupyter_core/application.py @@ -168,17 +168,17 @@ def ask(): def migrate_config(self): """Migrate config/data from IPython 3""" - try:# let's see if we can open the marker file + try: # let's see if we can open the marker file # for reading and updating (writing) f_marker = open(os.path.join(self.config_dir, "migrated"), 'r+') except PermissionError: # not readable and/or writable return # so let' give up migration in such an environment - except FileNotFoundError: # cannot find the marker file - pass # that means we have not migrated yet, so continue - else: # if we got here without raising anything, + except FileNotFoundError: # cannot find the marker file + pass # that means we have not migrated yet, so continue + else: # if we got here without raising anything, # that means the file exists f_marker.close() - return # so we must have already migrated -> bail out + return # so we must have already migrated -> bail out from .migrate import get_ipython_dir, migrate diff --git a/jupyter_core/tests/test_migrate.py b/jupyter_core/tests/test_migrate.py index 9deef89..45dcb09 100644 --- a/jupyter_core/tests/test_migrate.py +++ b/jupyter_core/tests/test_migrate.py @@ -11,6 +11,7 @@ import pytest from jupyter_core import migrate as migrate_mod +from jupyter_core.application import JupyterApp from jupyter_core.migrate import ( migrate, migrate_config, @@ -21,8 +22,6 @@ ) from jupyter_core.utils import ensure_dir_exists -from jupyter_core.application import JupyterApp - pjoin = os.path.join here = os.path.dirname(__file__) dotipython = pjoin(here, "dotipython") @@ -219,6 +218,7 @@ def test_migrate(env): assert os.path.exists(env["JUPYTER_CONFIG_DIR"]) assert os.path.exists(env["JUPYTER_DATA_DIR"]) + def test_app_migrate(env): shutil.copytree(dotipython, env["IPYTHONDIR"]) app = JupyterApp() @@ -240,7 +240,7 @@ def test_app_migrate_skip_unwritable_marker(env): shutil.copytree(dotipython, env["IPYTHONDIR"]) migrated_marker = pjoin(env["JUPYTER_CONFIG_DIR"], "migrated") touch(migrated_marker, "done") - os.chmod(migrated_marker, 0) # make it unworkable + os.chmod(migrated_marker, 0) # make it unworkable app = JupyterApp() app.initialize([]) assert os.listdir(env["JUPYTER_CONFIG_DIR"]) == ["migrated"] From 78ed3f8ff9c24c4a276c6c0af4ea367b50e814c3 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Wed, 14 Jun 2023 09:25:19 -0500 Subject: [PATCH 3/6] Update jupyter_core/application.py --- jupyter_core/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter_core/application.py b/jupyter_core/application.py index b100764..ad70bbc 100644 --- a/jupyter_core/application.py +++ b/jupyter_core/application.py @@ -172,7 +172,7 @@ def migrate_config(self): # for reading and updating (writing) f_marker = open(os.path.join(self.config_dir, "migrated"), 'r+') except PermissionError: # not readable and/or writable - return # so let' give up migration in such an environment + return # so let's give up migration in such an environment except FileNotFoundError: # cannot find the marker file pass # that means we have not migrated yet, so continue else: # if we got here without raising anything, From b8b9d0b09fa6e2c336971ec5e5a32a50d906360b Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Wed, 14 Jun 2023 09:26:43 -0500 Subject: [PATCH 4/6] lint --- jupyter_core/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter_core/application.py b/jupyter_core/application.py index ad70bbc..8b55a8a 100644 --- a/jupyter_core/application.py +++ b/jupyter_core/application.py @@ -170,7 +170,7 @@ def migrate_config(self): """Migrate config/data from IPython 3""" try: # let's see if we can open the marker file # for reading and updating (writing) - f_marker = open(os.path.join(self.config_dir, "migrated"), 'r+') + f_marker = open(os.path.join(self.config_dir, "migrated"), 'r+') # noqa except PermissionError: # not readable and/or writable return # so let's give up migration in such an environment except FileNotFoundError: # cannot find the marker file From 408d41f6eac0dccbc436787379544f89994b241a Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Wed, 14 Jun 2023 09:35:16 -0500 Subject: [PATCH 5/6] fix test on windows --- jupyter_core/tests/test_migrate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter_core/tests/test_migrate.py b/jupyter_core/tests/test_migrate.py index 45dcb09..9a6067f 100644 --- a/jupyter_core/tests/test_migrate.py +++ b/jupyter_core/tests/test_migrate.py @@ -54,7 +54,7 @@ def env(request): def fin(): """Cleanup test env""" env_patch.stop() - shutil.rmtree(td) + shutil.rmtree(td, ignore_errors=os.name=='nt') request.addfinalizer(fin) From 2d1d9d73f408c8fe46a5bce081ae6a03650b648a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 14 Jun 2023 14:35:33 +0000 Subject: [PATCH 6/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- jupyter_core/tests/test_migrate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter_core/tests/test_migrate.py b/jupyter_core/tests/test_migrate.py index 9a6067f..b86887e 100644 --- a/jupyter_core/tests/test_migrate.py +++ b/jupyter_core/tests/test_migrate.py @@ -54,7 +54,7 @@ def env(request): def fin(): """Cleanup test env""" env_patch.stop() - shutil.rmtree(td, ignore_errors=os.name=='nt') + shutil.rmtree(td, ignore_errors=os.name == 'nt') request.addfinalizer(fin)