From 3c84ba50a637c2cfc4183fe764aa0549d76dfeb2 Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Mon, 20 Feb 2023 09:13:27 +0000 Subject: [PATCH 01/11] wip --- cylc/uiserver/scripts/gui.py | 57 +++++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/cylc/uiserver/scripts/gui.py b/cylc/uiserver/scripts/gui.py index b20bb1ad..bbdc5e0b 100644 --- a/cylc/uiserver/scripts/gui.py +++ b/cylc/uiserver/scripts/gui.py @@ -30,6 +30,7 @@ import random import re import sys +from urllib import request import webbrowser @@ -50,6 +51,8 @@ def main(*argv): init_log() + import mdb + mdb.debug(ui_server=True) jp_server_opts, new_gui, workflow_id = parse_args_opts() if '--help' not in sys.argv: # get existing jpserver--open.html files @@ -57,21 +60,55 @@ def main(*argv): # these files are cleaned by jpserver on shutdown existing_guis = glob(os.path.join(INFO_FILES_DIR, "*open.html")) if existing_guis and not new_gui: - gui_file = random.choice(existing_guis) - print( - "Opening with existing gui." + - f" Use {CLI_OPT_NEW} option for a new gui.", - file=sys.stderr - ) - update_html_file(gui_file, workflow_id) - if '--no-browser' not in sys.argv: - webbrowser.open(f'file://{gui_file}', autoraise=True) - return + gui_file = select_info_file(existing_guis) + if gui_file: + print( + "Opening with existing gui." + + f" Use {CLI_OPT_NEW} option for a new gui.", + file=sys.stderr + ) + update_html_file(gui_file, workflow_id) + if '--no-browser' not in sys.argv: + webbrowser.open(f'file://{gui_file}', autoraise=True) + return return CylcUIServer.launch_instance( jp_server_opts or None, workflow_id=workflow_id ) +def select_info_file(existing_guis: list): + """This will select an active ui-server info file""" + existing_guis.sort(key=os.path.getmtime, reverse=True) + for gui_file in existing_guis: + if is_active_gui(gui_file): + return gui_file + check_remove_file(gui_file) + + +def is_active_gui(gui_file): + """Returns true if return code is 200 from server""" + if request.urlopen(f"file://{gui_file}").getcode() == 200: + return True + return False + + +def clean_info_file(gui_file): + try: + os.unlink(gui_file) + except Exception: + pass + + +def check_remove_file(gui_file) -> None: + """Ask user if they want to remove the file.""" + print(f"The following file cannot be used to open the Cylc GUI: {gui_file}.\n" + "The ui-server may be running on another host, or it may be down.\n") + ret = input('Do you want to remove this file? (y/n): ') + if ret.lower() == 'y': + clean_info_file(gui_file) + return + + def print_error(error: str, msg: str): """Print formatted error with message""" print(cparse( From 86da49d947f52bec0b6876606035feb805cce944 Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Wed, 22 Feb 2023 12:01:16 +0000 Subject: [PATCH 02/11] Add tests --- cylc/uiserver/scripts/gui.py | 78 +++++++++++++++--------------- cylc/uiserver/tests/test_gui.py | 84 +++++++++++++++++++++++++-------- 2 files changed, 104 insertions(+), 58 deletions(-) diff --git a/cylc/uiserver/scripts/gui.py b/cylc/uiserver/scripts/gui.py index bbdc5e0b..93c0207b 100644 --- a/cylc/uiserver/scripts/gui.py +++ b/cylc/uiserver/scripts/gui.py @@ -29,8 +29,9 @@ from pathlib import Path import random import re +from requests.exceptions import RequestException +import requests import sys -from urllib import request import webbrowser @@ -51,8 +52,6 @@ def main(*argv): init_log() - import mdb - mdb.debug(ui_server=True) jp_server_opts, new_gui, workflow_id = parse_args_opts() if '--help' not in sys.argv: # get existing jpserver--open.html files @@ -60,41 +59,56 @@ def main(*argv): # these files are cleaned by jpserver on shutdown existing_guis = glob(os.path.join(INFO_FILES_DIR, "*open.html")) if existing_guis and not new_gui: - gui_file = select_info_file(existing_guis) - if gui_file: + url = select_info_file(existing_guis) + if url: print( "Opening with existing gui." + f" Use {CLI_OPT_NEW} option for a new gui.", file=sys.stderr ) - update_html_file(gui_file, workflow_id) + url = update_url(url, workflow_id) if '--no-browser' not in sys.argv: - webbrowser.open(f'file://{gui_file}', autoraise=True) + webbrowser.open(url, autoraise=True) return return CylcUIServer.launch_instance( jp_server_opts or None, workflow_id=workflow_id ) -def select_info_file(existing_guis: list): +def select_info_file(existing_guis: list) -> str: """This will select an active ui-server info file""" existing_guis.sort(key=os.path.getmtime, reverse=True) for gui_file in existing_guis: - if is_active_gui(gui_file): - return gui_file + url = fish_url_from_file(gui_file) + if url and is_active_gui(url): + return url check_remove_file(gui_file) -def is_active_gui(gui_file): +def fish_url_from_file(gui_file): + with open(gui_file, "r") as f: + file_content = f.read() + url_extract_regex = re.compile('url=(.*?)\"') + match = url_extract_regex.search(file_content) + return match.group(1) if match else None + + +def is_active_gui(url): """Returns true if return code is 200 from server""" - if request.urlopen(f"file://{gui_file}").getcode() == 200: - return True - return False + try: + req = requests.get(url) + if req.status_code == 200: + return True + except RequestException: + return False -def clean_info_file(gui_file): +def clean_info_files(gui_file): + pid = re.compile('-(\d*)-open\.html').search(gui_file).group(1) + json_file = os.path.join(INFO_FILES_DIR, f"jpserver-{pid}.json") try: os.unlink(gui_file) + os.unlink(json_file) except Exception: pass @@ -102,10 +116,10 @@ def clean_info_file(gui_file): def check_remove_file(gui_file) -> None: """Ask user if they want to remove the file.""" print(f"The following file cannot be used to open the Cylc GUI: {gui_file}.\n" - "The ui-server may be running on another host, or it may be down.\n") + "The ui-server may be running on another host, or it may be down.") ret = input('Do you want to remove this file? (y/n): ') if ret.lower() == 'y': - clean_info_file(gui_file) + clean_info_files(gui_file) return @@ -172,43 +186,29 @@ def get_arg_parser(): return parser -def update_html_file(gui_file, workflow_id): - """ Update the html file to open at the correct workflow in the gui. +def update_url(url, workflow_id): + """ Update the url to open at the correct workflow in the gui. """ - with open(gui_file, "r") as f: - file_content = f.read() - url_extract_regex = re.compile('url=(.*?)\"') - url_string = url_extract_regex.search(file_content) - if not url_string: + if not url: return - url = url_string.group(1) split_url = url.split('/workspace/') if not workflow_id: # new url should point to dashboard if len(split_url) == 1: # no update required - return + return url else: # previous url points to workflow page and needs updating # replace with base url (including token) - replacement_url_string = split_url[0] + return split_url[0] else: if len(split_url) > 1: old_workflow = split_url[1] if workflow_id == old_workflow: # same workflow page requested, no update needed - return + return url else: - replacement_url_string = url.replace(old_workflow, workflow_id) + return url.replace(old_workflow, workflow_id) else: # current url points to dashboard, update to point to workflow - replacement_url_string = f"{url}/workspace/{workflow_id}" - update_url_string(gui_file, url, replacement_url_string) - - -def update_url_string(gui_file: str, url: str, replacement_url_string: str): - """Updates the url string in the given gui file.""" - file = Path(gui_file) - current_text = file.read_text() - updated_text = current_text.replace(url, replacement_url_string) - file.write_text(updated_text) + return f"{url}/workspace/{workflow_id}" diff --git a/cylc/uiserver/tests/test_gui.py b/cylc/uiserver/tests/test_gui.py index c42d4604..60eccb1d 100644 --- a/cylc/uiserver/tests/test_gui.py +++ b/cylc/uiserver/tests/test_gui.py @@ -13,44 +13,48 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +from glob import glob import json +import mock +import os from pathlib import Path import pytest -import os +from random import randint +from time import sleep -from cylc.uiserver.scripts.gui import update_html_file +from cylc.uiserver.scripts.gui import fish_url_from_file, select_info_file, update_url @pytest.mark.parametrize( 'existing_content,workflow_id,expected_updated_content', [ pytest.param( - 'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#" /> ', + 'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#', None, - 'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#" /> ', + 'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#', id='existing_no_workflow_new_no_workflow' ), pytest.param( - 'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#" /> ', + 'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#', 'some/workflow', - 'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow" /> ', + 'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow', id='existing_no_workflow_new_workflow' ), pytest.param( - 'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow" /> ', + 'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow', 'another/flow', - 'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/another/flow" /> ', + 'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/another/flow', id='existing_workflow_new_workflow' ), pytest.param( - 'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow" /> ', + 'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow', None, - 'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#" /> ', + 'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#', id='existing_workflow_no_new_workflow' ), pytest.param( - 'content="1;no url in this file "', + '', 'another/flow', - 'content="1;no url in this file "', + None, id='no_url_no_change' ), ] @@ -58,14 +62,56 @@ def test_update_html_file_updates_gui_file( existing_content, workflow_id, - expected_updated_content, - tmp_path): - """Tests html file is updated correctly""" + expected_updated_content): + """Tests url is updated correctly""" + + updated_file_content = update_url(existing_content, workflow_id) + assert updated_file_content == expected_updated_content + + +@pytest.mark.parametrize( + 'file_content,expected_url', + [ + pytest.param( + 'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow" /> ', + "http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow", + id='url_in_file' + ), + pytest.param( + 'There is no url in here', + None, + id='no_url_in_file' + ), + ] +) +def test_fish_url_from_file(file_content, expected_url, tmp_path): Path(tmp_path).mkdir(exist_ok=True) tmp_gui_file = Path(tmp_path / "gui") tmp_gui_file.touch() - tmp_gui_file.write_text(existing_content) - update_html_file(tmp_gui_file, workflow_id) - updated_file_content = tmp_gui_file.read_text() + tmp_gui_file.write_text(file_content) + actual_url = fish_url_from_file(tmp_gui_file) + assert actual_url == expected_url - assert updated_file_content == expected_updated_content + +def test_gui_selection_and_clean_process(tmp_path, monkeypatch): + # set up file structure + info_files_dir = Path(tmp_path/'.cylc'/'uiserver'/'info_files') + info_files_dir.mkdir(parents=True, exist_ok=True) + for i in range(1, 5): + pid = randint(1000, 100000) + html_file = (info_files_dir / f"jpserver-{pid}-open.html") + json_file = (info_files_dir / f"jpserver-{pid}.json") + html_file.touch() + html_file.write_text(f"") + # Sleep ensure different modification time for sort + sleep(0.1) + mock_existing_guis = glob(os.path.join(info_files_dir, "*open.html")) + # with mock.patch.object(__builtins__, 'input', lambda: 'y'): + monkeypatch.setattr( + 'cylc.uiserver.gui.check_remove_file.input', + 'y') + monkeypatch.setattr( + 'cylc.uiserver.gui.is_active_gui.re', + 'y') + url = select_info_file(mock_existing_guis) + From 15ac360c4c44acc0fda769bd1bb2bba24d0ef24c Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Wed, 1 Mar 2023 16:02:44 +0000 Subject: [PATCH 03/11] Add tests for info files --- cylc/uiserver/scripts/gui.py | 6 ++- cylc/uiserver/tests/test_gui.py | 72 +++++++++++++++++++++++++++------ 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/cylc/uiserver/scripts/gui.py b/cylc/uiserver/scripts/gui.py index 93c0207b..e73728f3 100644 --- a/cylc/uiserver/scripts/gui.py +++ b/cylc/uiserver/scripts/gui.py @@ -55,8 +55,10 @@ def main(*argv): jp_server_opts, new_gui, workflow_id = parse_args_opts() if '--help' not in sys.argv: # get existing jpserver--open.html files - # assume if this exists then the server is still running - # these files are cleaned by jpserver on shutdown + # check if the server is available for use + # prompt for user whether to clean files for un-usable uiservers + # these files are usually cleaned by jpserver on shutdown, although + # can be left behind on crash or a `kill -9` of the process existing_guis = glob(os.path.join(INFO_FILES_DIR, "*open.html")) if existing_guis and not new_gui: url = select_info_file(existing_guis) diff --git a/cylc/uiserver/tests/test_gui.py b/cylc/uiserver/tests/test_gui.py index 60eccb1d..4778a2f6 100644 --- a/cylc/uiserver/tests/test_gui.py +++ b/cylc/uiserver/tests/test_gui.py @@ -13,16 +13,21 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import builtins from glob import glob -import json -import mock import os from pathlib import Path import pytest from random import randint +import requests +from shutil import rmtree from time import sleep -from cylc.uiserver.scripts.gui import fish_url_from_file, select_info_file, update_url +from cylc.uiserver.scripts.gui import ( + fish_url_from_file, + select_info_file, + update_url +) @pytest.mark.parametrize( 'existing_content,workflow_id,expected_updated_content', @@ -91,27 +96,70 @@ def test_fish_url_from_file(file_content, expected_url, tmp_path): tmp_gui_file.write_text(file_content) actual_url = fish_url_from_file(tmp_gui_file) assert actual_url == expected_url + rmtree(tmp_path, ignore_errors=True) def test_gui_selection_and_clean_process(tmp_path, monkeypatch): + """Testing functionally the gui selection and cleaning process""" # set up file structure info_files_dir = Path(tmp_path/'.cylc'/'uiserver'/'info_files') info_files_dir.mkdir(parents=True, exist_ok=True) for i in range(1, 5): pid = randint(1000, 100000) html_file = (info_files_dir / f"jpserver-{pid}-open.html") - json_file = (info_files_dir / f"jpserver-{pid}.json") + # the json file is unused but created empty to ensure the html is the + # file used for selection + _json_file = (info_files_dir / f"jpserver-{pid}.json") html_file.touch() - html_file.write_text(f"") + html_file.write_text(f"content=\"1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token{pid}#/workspace/some/workflow\" more content") # Sleep ensure different modification time for sort sleep(0.1) mock_existing_guis = glob(os.path.join(info_files_dir, "*open.html")) - # with mock.patch.object(__builtins__, 'input', lambda: 'y'): - monkeypatch.setattr( - 'cylc.uiserver.gui.check_remove_file.input', - 'y') + monkeypatch.setattr(requests, 'get', mock_get) + url = select_info_file(mock_existing_guis) + # Test that the most recent ui-server is selected: + assert url == f"http://localhost:8892/cylc/?token=1234567890some_big_long_token{pid}#/workspace/some/workflow" + rmtree(tmp_path, ignore_errors=True) + + +def test_cleaning_of_info_files(tmp_path, monkeypatch): + """Functionally tests the cleaning logic of the info files""" + mock_info_files_dir = Path(tmp_path/'.cylc'/'uiserver'/'info_files') + mock_info_files_dir.mkdir(parents=True, exist_ok=True) + html_file = (mock_info_files_dir / f"jpserver-12345-open.html") + json_file = (mock_info_files_dir / f"jpserver-12345.json") + json_file.touch() + html_file.touch() + html_file.write_text(f"Some content but no url in here") + assert html_file.exists() is True + assert json_file.exists() is True + # assert is called + mock_existing_guis = glob(os.path.join(mock_info_files_dir, "*open.html")) + monkeypatch.setattr(builtins, 'input', lambda *args, **kwargs: 'n') + # test that a no user response keeps the files + url = select_info_file(mock_existing_guis) + assert url is None + assert html_file.exists() is True + assert json_file.exists() is True + # Change user response to a yes and test files are removed + monkeypatch.setattr(builtins, 'input', lambda *args, **kwargs: 'y') monkeypatch.setattr( - 'cylc.uiserver.gui.is_active_gui.re', - 'y') + 'cylc.uiserver.scripts.gui.INFO_FILES_DIR', + mock_info_files_dir + ) url = select_info_file(mock_existing_guis) - + assert url is None + # test clean takes place + assert html_file.exists() is False + assert json_file.exists() is False + rmtree(tmp_path, ignore_errors=True) + + +class MockResponse: + """Used for the response of mocked request""" + def __init__(self): + self.status_code = 200 + + +def mock_get(*args, **kwargs): + return MockResponse() From 7bd108a86ed9cb5f0e30c0d4a8031d670e65997b Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Thu, 2 Mar 2023 09:56:50 +0000 Subject: [PATCH 04/11] flake8 --- cylc/uiserver/scripts/gui.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cylc/uiserver/scripts/gui.py b/cylc/uiserver/scripts/gui.py index e73728f3..e2ea74f7 100644 --- a/cylc/uiserver/scripts/gui.py +++ b/cylc/uiserver/scripts/gui.py @@ -26,8 +26,6 @@ from contextlib import suppress from glob import glob import os -from pathlib import Path -import random import re from requests.exceptions import RequestException import requests @@ -106,7 +104,7 @@ def is_active_gui(url): def clean_info_files(gui_file): - pid = re.compile('-(\d*)-open\.html').search(gui_file).group(1) + pid = re.compile(r'-(\d*)-open\.html').search(gui_file).group(1) json_file = os.path.join(INFO_FILES_DIR, f"jpserver-{pid}.json") try: os.unlink(gui_file) @@ -117,8 +115,9 @@ def clean_info_files(gui_file): def check_remove_file(gui_file) -> None: """Ask user if they want to remove the file.""" - print(f"The following file cannot be used to open the Cylc GUI: {gui_file}.\n" - "The ui-server may be running on another host, or it may be down.") + print("The following file cannot be used to open the Cylc GUI:" + f" {gui_file}.\nThe ui-server may be running on another host," + " or it may be down.") ret = input('Do you want to remove this file? (y/n): ') if ret.lower() == 'y': clean_info_files(gui_file) From e6a6979dec81350711b0071aff5bd8fd80e80129 Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Thu, 2 Mar 2023 11:19:56 +0000 Subject: [PATCH 05/11] mypy --- cylc/uiserver/scripts/gui.py | 4 +++- setup.cfg | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/cylc/uiserver/scripts/gui.py b/cylc/uiserver/scripts/gui.py index e2ea74f7..1a013a02 100644 --- a/cylc/uiserver/scripts/gui.py +++ b/cylc/uiserver/scripts/gui.py @@ -30,6 +30,7 @@ from requests.exceptions import RequestException import requests import sys +from typing import Optional import webbrowser @@ -75,7 +76,7 @@ def main(*argv): ) -def select_info_file(existing_guis: list) -> str: +def select_info_file(existing_guis: list) -> Optional[str]: """This will select an active ui-server info file""" existing_guis.sort(key=os.path.getmtime, reverse=True) for gui_file in existing_guis: @@ -83,6 +84,7 @@ def select_info_file(existing_guis: list) -> str: if url and is_active_gui(url): return url check_remove_file(gui_file) + return None def fish_url_from_file(gui_file): diff --git a/setup.cfg b/setup.cfg index 485d27e7..62bef4ab 100644 --- a/setup.cfg +++ b/setup.cfg @@ -99,6 +99,7 @@ tests = pytest-tornasync>=0.5.0 pytest>=6 types-pkg_resources>=0.1.2 + types-requests>2 all = %(hub)s %(tests)s From fae1911c89b9579e9757324bacf18f5ab359307b Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Mon, 6 Mar 2023 13:11:01 +0000 Subject: [PATCH 06/11] Add requests as depencency --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 62bef4ab..8b0c8cc4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -56,6 +56,7 @@ install_requires = graphene-tornado==2.6.* graphql-ws==0.4.4 jupyter_server>=1.10.2 + requests tornado>=6.1.0 # matches jupyter_server value traitlets>=5.2.1 # required for logging_config (5.2.0 had bugs) From 3a38df1f088bce6d6a08d5747a4f583e0563666d Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Thu, 16 Mar 2023 10:43:01 +0000 Subject: [PATCH 07/11] Respond to Review Feedback TP --- cylc/uiserver/scripts/gui.py | 7 ++++--- cylc/uiserver/tests/test_gui.py | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/cylc/uiserver/scripts/gui.py b/cylc/uiserver/scripts/gui.py index 1a013a02..7b81a67f 100644 --- a/cylc/uiserver/scripts/gui.py +++ b/cylc/uiserver/scripts/gui.py @@ -80,14 +80,14 @@ def select_info_file(existing_guis: list) -> Optional[str]: """This will select an active ui-server info file""" existing_guis.sort(key=os.path.getmtime, reverse=True) for gui_file in existing_guis: - url = fish_url_from_file(gui_file) + url = get_url_from_file(gui_file) if url and is_active_gui(url): return url check_remove_file(gui_file) return None -def fish_url_from_file(gui_file): +def get_url_from_file(gui_file): with open(gui_file, "r") as f: file_content = f.read() url_extract_regex = re.compile('url=(.*?)\"') @@ -120,7 +120,8 @@ def check_remove_file(gui_file) -> None: print("The following file cannot be used to open the Cylc GUI:" f" {gui_file}.\nThe ui-server may be running on another host," " or it may be down.") - ret = input('Do you want to remove this file? (y/n): ') + ret = input( + cparse('Do you want to remove this file? (y/n):')) if ret.lower() == 'y': clean_info_files(gui_file) return diff --git a/cylc/uiserver/tests/test_gui.py b/cylc/uiserver/tests/test_gui.py index 4778a2f6..7bae7e01 100644 --- a/cylc/uiserver/tests/test_gui.py +++ b/cylc/uiserver/tests/test_gui.py @@ -24,7 +24,7 @@ from time import sleep from cylc.uiserver.scripts.gui import ( - fish_url_from_file, + get_url_from_file, select_info_file, update_url ) @@ -89,12 +89,12 @@ def test_update_html_file_updates_gui_file( ), ] ) -def test_fish_url_from_file(file_content, expected_url, tmp_path): +def test_get_url_from_file(file_content, expected_url, tmp_path): Path(tmp_path).mkdir(exist_ok=True) tmp_gui_file = Path(tmp_path / "gui") tmp_gui_file.touch() tmp_gui_file.write_text(file_content) - actual_url = fish_url_from_file(tmp_gui_file) + actual_url = get_url_from_file(tmp_gui_file) assert actual_url == expected_url rmtree(tmp_path, ignore_errors=True) From 079637262492250f448bfa59bebc3a6fe8abe28c Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Thu, 16 Mar 2023 13:12:48 +0000 Subject: [PATCH 08/11] change log --- CHANGES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 884fa2f5..01c9a88f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -23,6 +23,12 @@ the log file now captures messages from cylc-flow. [#370](https://github.com/cylc/cylc-uiserver/pull/370) - `cylc gui workflow_id` is now supported and will open the GUI at that workflow. +### Fixes + +[#431](https://github.com/cylc/cylc-uiserver/pull/431)- Adds an additional +cleaning check for the ui-server contact file. This may have caused problems +running a ui-server following a crash. + ------------------------------------------------------------------------------- ## __cylc-uiserver-1.2.0 (Released 2023-01-16)__ From c06906bccd673be43fed357d09e36f97f663cc43 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Thu, 30 Mar 2023 15:29:52 +0100 Subject: [PATCH 09/11] Update changelog --- CHANGES.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 8b11de72..d8edcc8c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,17 @@ creating a new release entry be sure to copy & paste the span tag with the `actions:bind` attribute, which is used by a regex to find the text to be updated. Only the first match gets replaced, so it's fine to leave the old ones in. --> +------------------------------------------------------------------------------- +## __cylc-uiserver-1.2.2 (Upcoming)__ + + + +### Fixes + +[#431](https://github.com/cylc/cylc-uiserver/pull/431)- Adds an additional +cleaning check for the UI server contact file. This may have caused problems +running a UI server following a crash. + ------------------------------------------------------------------------------- ## __cylc-uiserver-1.2.1 (Released 2023-02-20)__ @@ -23,12 +34,6 @@ the log file now captures messages from cylc-flow. [#370](https://github.com/cylc/cylc-uiserver/pull/370) - `cylc gui workflow_id` is now supported and will open the GUI at that workflow. -### Fixes - -[#431](https://github.com/cylc/cylc-uiserver/pull/431)- Adds an additional -cleaning check for the ui-server contact file. This may have caused problems -running a ui-server following a crash. - ------------------------------------------------------------------------------- ## __cylc-uiserver-1.2.0 (Released 2023-01-16)__ From 6e87a39e6c9845ecc3aa39a4ce8559970793ec6f Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 31 Mar 2023 10:30:09 +0100 Subject: [PATCH 10/11] Apply suggestions from code review Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- cylc/uiserver/scripts/gui.py | 5 ++--- cylc/uiserver/tests/test_gui.py | 6 ++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/cylc/uiserver/scripts/gui.py b/cylc/uiserver/scripts/gui.py index 7b81a67f..a53760a6 100644 --- a/cylc/uiserver/scripts/gui.py +++ b/cylc/uiserver/scripts/gui.py @@ -108,11 +108,10 @@ def is_active_gui(url): def clean_info_files(gui_file): pid = re.compile(r'-(\d*)-open\.html').search(gui_file).group(1) json_file = os.path.join(INFO_FILES_DIR, f"jpserver-{pid}.json") - try: + with suppress(OSError): os.unlink(gui_file) + with suppress(OSError): os.unlink(json_file) - except Exception: - pass def check_remove_file(gui_file) -> None: diff --git a/cylc/uiserver/tests/test_gui.py b/cylc/uiserver/tests/test_gui.py index 7bae7e01..3628c699 100644 --- a/cylc/uiserver/tests/test_gui.py +++ b/cylc/uiserver/tests/test_gui.py @@ -96,7 +96,6 @@ def test_get_url_from_file(file_content, expected_url, tmp_path): tmp_gui_file.write_text(file_content) actual_url = get_url_from_file(tmp_gui_file) assert actual_url == expected_url - rmtree(tmp_path, ignore_errors=True) def test_gui_selection_and_clean_process(tmp_path, monkeypatch): @@ -109,7 +108,8 @@ def test_gui_selection_and_clean_process(tmp_path, monkeypatch): html_file = (info_files_dir / f"jpserver-{pid}-open.html") # the json file is unused but created empty to ensure the html is the # file used for selection - _json_file = (info_files_dir / f"jpserver-{pid}.json") + json_file = (info_files_dir / f"jpserver-{pid}.json") + json_file.touch() html_file.touch() html_file.write_text(f"content=\"1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token{pid}#/workspace/some/workflow\" more content") # Sleep ensure different modification time for sort @@ -119,7 +119,6 @@ def test_gui_selection_and_clean_process(tmp_path, monkeypatch): url = select_info_file(mock_existing_guis) # Test that the most recent ui-server is selected: assert url == f"http://localhost:8892/cylc/?token=1234567890some_big_long_token{pid}#/workspace/some/workflow" - rmtree(tmp_path, ignore_errors=True) def test_cleaning_of_info_files(tmp_path, monkeypatch): @@ -152,7 +151,6 @@ def test_cleaning_of_info_files(tmp_path, monkeypatch): # test clean takes place assert html_file.exists() is False assert json_file.exists() is False - rmtree(tmp_path, ignore_errors=True) class MockResponse: From e8c5cd3dfcb42a155310be0fee8630ebac7d47c2 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 31 Mar 2023 10:30:29 +0100 Subject: [PATCH 11/11] Update cylc/uiserver/scripts/gui.py --- cylc/uiserver/scripts/gui.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cylc/uiserver/scripts/gui.py b/cylc/uiserver/scripts/gui.py index a53760a6..688d5c57 100644 --- a/cylc/uiserver/scripts/gui.py +++ b/cylc/uiserver/scripts/gui.py @@ -95,12 +95,10 @@ def get_url_from_file(gui_file): return match.group(1) if match else None -def is_active_gui(url): +def is_active_gui(url: str) -> bool: """Returns true if return code is 200 from server""" try: - req = requests.get(url) - if req.status_code == 200: - return True + return requests.get(url).status_code == 200 except RequestException: return False