diff --git a/CHANGES.md b/CHANGES.md index 14267bd8..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)__ diff --git a/cylc/uiserver/scripts/gui.py b/cylc/uiserver/scripts/gui.py index b20bb1ad..688d5c57 100644 --- a/cylc/uiserver/scripts/gui.py +++ b/cylc/uiserver/scripts/gui.py @@ -26,10 +26,11 @@ 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 import sys +from typing import Optional import webbrowser @@ -53,25 +54,76 @@ 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: - 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 + 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 + ) + url = update_url(url, workflow_id) + if '--no-browser' not in sys.argv: + 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) -> 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 = get_url_from_file(gui_file) + if url and is_active_gui(url): + return url + check_remove_file(gui_file) + return None + + +def get_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: str) -> bool: + """Returns true if return code is 200 from server""" + try: + return requests.get(url).status_code == 200 + except RequestException: + return False + + +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") + with suppress(OSError): + os.unlink(gui_file) + with suppress(OSError): + os.unlink(json_file) + + +def check_remove_file(gui_file) -> None: + """Ask user if they want to remove the file.""" + 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( + cparse('Do you want to remove this file? (y/n):')) + if ret.lower() == 'y': + clean_info_files(gui_file) + return + + def print_error(error: str, msg: str): """Print formatted error with message""" print(cparse( @@ -135,43 +187,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..3628c699 100644 --- a/cylc/uiserver/tests/test_gui.py +++ b/cylc/uiserver/tests/test_gui.py @@ -13,44 +13,53 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import json +import builtins +from glob import glob +import os from pathlib import Path import pytest -import os +from random import randint +import requests +from shutil import rmtree +from time import sleep -from cylc.uiserver.scripts.gui import update_html_file +from cylc.uiserver.scripts.gui import ( + get_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 +67,97 @@ 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_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(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 = get_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): + """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") + # 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.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 + sleep(0.1) + mock_existing_guis = glob(os.path.join(info_files_dir, "*open.html")) + 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" + + +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.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 + + +class MockResponse: + """Used for the response of mocked request""" + def __init__(self): + self.status_code = 200 + + +def mock_get(*args, **kwargs): + return MockResponse() diff --git a/setup.cfg b/setup.cfg index fe4a281c..5e354aa0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -57,6 +57,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) @@ -100,6 +101,7 @@ tests = pytest-tornasync>=0.5.0 pytest>=6 types-pkg_resources>=0.1.2 + types-requests>2 all = %(hub)s %(tests)s