From 0016010378b283f3f3b75c0535bff4bc8d92db03 Mon Sep 17 00:00:00 2001 From: Emma Date: Thu, 12 Jun 2025 15:33:12 +0100 Subject: [PATCH 01/36] new branch --- src/packit_client.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/packit_client.py diff --git a/src/packit_client.py b/src/packit_client.py new file mode 100644 index 0000000..b553e3f --- /dev/null +++ b/src/packit_client.py @@ -0,0 +1,13 @@ +import montagu + +# TODO: How will we be able to recognised expired token? + +class PackitClient: + def __init__(self, config): + self.config = config + + #def __get() + #def __post() + #def run_packet() + #def poll_running_packet() + #def kill_running_packet() \ No newline at end of file From 4c7e3ce87ccd364357526a2441f8acd06396849f Mon Sep 17 00:00:00 2001 From: Emma Date: Thu, 12 Jun 2025 16:22:44 +0100 Subject: [PATCH 02/36] remove orderly web install, fix docker compose --- README.md | 2 +- config/docker_config.yml | 2 +- docker-compose.yml | 4 ++-- scripts/run-dependencies.sh | 22 +++++++++------------- src/packit_client.py | 3 ++- 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index eb36d10..0f36444 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ The worker expects to find a config file at `config/config.yml`. The Dockerfile copies `config/docker_config.yml` to `config/config.yml`. This allows the worker running on metal to use a broker on `localhost` while the worker in docker needs to use -`montagu_mq`, the container name of the broker, to access its port. +`montagu-mq`, the container name of the broker, to access its port. Note that if a YouTrack token is not provided in the config the app will look for an environment variable called `YOUTRACK_TOKEN`. This makes local and automated testing of the YouTrack integration possible. diff --git a/config/docker_config.yml b/config/docker_config.yml index 0de73b9..2688eae 100644 --- a/config/docker_config.yml +++ b/config/docker_config.yml @@ -1,4 +1,4 @@ -host: montagu_mq_1 +host: montagu-mq-1 servers: montagu: url: http://montagu_api_1:8080 diff --git a/docker-compose.yml b/docker-compose.yml index 66087e6..484fcfe 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -9,8 +9,8 @@ services: ports: - "5555:5555" environment: - - CELERY_BROKER_URL=redis://montagu_mq_1// - - CELERY_RESULT_BACKEND=redis://montagu_mq_1/0 + - CELERY_BROKER_URL=redis://montagu-mq-1// + - CELERY_RESULT_BACKEND=redis://montagu-mq-1/0 - FLOWER_PORT=5555 api: image: ${REGISTRY}/montagu-api:master diff --git a/scripts/run-dependencies.sh b/scripts/run-dependencies.sh index 2f55700..12f0d35 100755 --- a/scripts/run-dependencies.sh +++ b/scripts/run-dependencies.sh @@ -10,23 +10,18 @@ export REGISTRY=vimc export NETWORK=montagu_default # Run the API and database -docker-compose pull -docker-compose --project-name montagu up -d +docker compose pull +docker compose --project-name montagu up -d # Clear redis -docker exec montagu_mq_1 redis-cli FLUSHALL - -# Install orderly-web -pip3 install constellation -pip3 install orderly-web -orderly-web start $here +docker exec montagu-mq-1 redis-cli FLUSHALL # Start the APIs -docker exec montagu_api_1 mkdir -p /etc/montagu/api/ -docker exec montagu_api_1 touch /etc/montagu/api/go_signal +docker exec montagu-api-1 mkdir -p /etc/montagu/api/ +docker exec montagu-api-1 touch /etc/montagu/api/go_signal # Wait for the database -docker exec montagu_db_1 montagu-wait.sh +docker exec montagu-db-1 montagu-wait.sh # migrate the database migrate_image=${REGISTRY}/montagu-migrate:master @@ -40,6 +35,7 @@ $here/montagu_cli.sh add "Test User" test.user \ $here/montagu_cli.sh addRole test.user user +# TODO: Add user to Packit # Add user to orderlyweb -$here/orderlyweb_cli.sh add-users test.user@example.com -$here/orderlyweb_cli.sh grant test.user@example.com */reports.read */reports.run */reports.review +#$here/orderlyweb_cli.sh add-users test.user@example.com +#$here/orderlyweb_cli.sh grant test.user@example.com */reports.read */reports.run */reports.review diff --git a/src/packit_client.py b/src/packit_client.py index b553e3f..9e7af6a 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -1,6 +1,7 @@ import montagu -# TODO: How will we be able to recognised expired token? +# TODO: How will we be able to recognise expired token? +# TODO: How to authenticate? user and pw from vault or something smarter? class PackitClient: def __init__(self, config): From 4cd236097fbd928fa214d66cafe23ec5663bc3cc Mon Sep 17 00:00:00 2001 From: Emma Date: Thu, 3 Jul 2025 10:54:04 +0100 Subject: [PATCH 03/36] add basic packit client --- src/packit_client.py | 78 ++++++++++++++++++++++++++++++---- src/packit_client_exception.py | 10 +++++ 2 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 src/packit_client_exception.py diff --git a/src/packit_client.py b/src/packit_client.py index 9e7af6a..4905ee6 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -1,14 +1,74 @@ +import requests import montagu - -# TODO: How will we be able to recognise expired token? -# TODO: How to authenticate? user and pw from vault or something smarter? +from .packit_client_exception import PackitClientException class PackitClient: def __init__(self, config): - self.config = config + self.__config = config + self.__authenticate() + + def __url(self, relative_url): + # TODO: add base url to config + return f"{self.__config.packit_api_url}{relative_url}" + + @staticmethod + def handle_response(response): + if response.status_code != 200 and response.status_code != 204: + raise PackitClientException(response) + return response.json() + + def __get(self, relative_url, headers = None): + if headers is None: + headers = self.__default_headers + response = requests.get(self.__url(relative_url), headers=headers) + return handle_response(response) + + def __post(self, relative_url, data, headers = None): + if headers is None: + headers = self.__default_headers + response requests.post(self.__url(relative_url, data=data, headers=headers) + return handle_response(response) + + def __authenticate(self): + try: + monty = montagu.MontaguAPI(self.__config.montagu_url, self.__config.montagu_user, + config.montagu_password) + packit_login_response = self.__get("/auth/login/montagu", {"Authorization": f"Bearer {monty.token}"}) + self.token = packit_login_response.token # TODO: maybe don't need to retain token as saving header? + self.__default_headers = { "Authorization": f"Bearer {self.token}" } + except Exception as ex: + logging.exception(ex) + + def __execute(self, func, *args): # TODO: maybe don't need args? + # retry an operation if it fails auth (probably because of an expired packit token) + try: + return func(*args) + except PackitClientException as ex: + if ex.response.status_code == 401: + self.__authenticate() + return func(*args) + else: + raise ex + + def run_packet(self, packet_group, parameters): + def do_run_packet(): + data = { + "name": packet_group, + "parameters": parameters, + "branch": "", + "hash": "" # TODO: can branch and hash be empty?? + } + response = self.__post("/runner/run") + return response.taskId + + return __self.execute(do_run_packet) + + def poll_running_packet(self, task_id): + def do_poll_running_packet(): + return self.__get(f"/runner/status/{task_id}") + return self.__execute(do_poll_running_packet) - #def __get() - #def __post() - #def run_packet() - #def poll_running_packet() - #def kill_running_packet() \ No newline at end of file + def kill_running_packet(self, task_id): + def do_kill_running_packet(): + return self.__post(f"/runner/cancel/{task_id}") + return self.execute(do_kill_running_packet) \ No newline at end of file diff --git a/src/packit_client_exception.py b/src/packit_client_exception.py new file mode 100644 index 0000000..6812ca7 --- /dev/null +++ b/src/packit_client_exception.py @@ -0,0 +1,10 @@ + + +class PackitClientException(Exception): + def __init__(self, response): + self.response = response + json = response.json() + msg = f"Unexpected response status from Packit API: {response.status_code}." + if "error" in json and "detail" in json["error"]: + msg = f"{msg} Detail: {json["error"]["detail"]}" + super(Exception, self).__init(msg) \ No newline at end of file From 93b3e999ca31ca3b373e2cd74d26039c7b069a62 Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 7 Jul 2025 16:40:22 +0100 Subject: [PATCH 04/36] add "publish" --- config/config.yml | 9 ++- scripts/packit.yml | 1 + scripts/run-dependencies.sh | 5 ++ src/config.py | 8 ++- src/packit_client.py | 60 ++++++++++++++----- src/task_run_diagnostic_reports.py | 6 +- src/utils/run_reports.py | 96 +++++++++++++++++++----------- 7 files changed, 128 insertions(+), 57 deletions(-) create mode 100644 scripts/packit.yml diff --git a/config/config.yml b/config/config.yml index eb769fd..ba70f9c 100644 --- a/config/config.yml +++ b/config/config.yml @@ -4,8 +4,8 @@ servers: url: http://localhost:8080 user: test.user@example.com password: password - orderlyweb: - url: http://localhost:8888 + packit: + url: http://localhost:3000 youtrack: token: None smtp: @@ -29,6 +29,9 @@ tasks: subject: "VIMC diagnostic report: {touchstone} - {group} - {disease}" timeout: 300 assignee: a.hill + publish_roles: + - minimal.modeller + - Funders - report_name: diagnostic-param parameters: nmin: 0 @@ -39,5 +42,7 @@ tasks: subject: "New version of another Orderly report" timeout: 1200 assignee: e.russell + publish_roles: + - other.modeller archive_folder_contents: min_file_age_seconds: 0 diff --git a/scripts/packit.yml b/scripts/packit.yml new file mode 100644 index 0000000..67769e0 --- /dev/null +++ b/scripts/packit.yml @@ -0,0 +1 @@ +# TODO: this should largely match uat/basicauth configs - but needs runner! \ No newline at end of file diff --git a/scripts/run-dependencies.sh b/scripts/run-dependencies.sh index 12f0d35..18011a4 100755 --- a/scripts/run-dependencies.sh +++ b/scripts/run-dependencies.sh @@ -16,6 +16,11 @@ docker compose --project-name montagu up -d # Clear redis docker exec montagu-mq-1 redis-cli FLUSHALL +# Install packit +pip3 install constellation +pip3 install packit-deploy +packit start --pull $here + # Start the APIs docker exec montagu-api-1 mkdir -p /etc/montagu/api/ docker exec montagu-api-1 touch /etc/montagu/api/go_signal diff --git a/src/config.py b/src/config.py index 5d9cf8e..946052a 100644 --- a/src/config.py +++ b/src/config.py @@ -9,13 +9,15 @@ def __init__(self, success_email_recipients, success_email_subject, timeout, - assignee): + assignee, + publish_roles): self.name = name self.parameters = parameters self.success_email_recipients = success_email_recipients self.success_email_subject = success_email_subject self.timeout = timeout self.assignee = assignee + self.publish_roles = publish_roles class ArchiveFolderContentsConfig: @@ -98,13 +100,15 @@ def diagnostic_reports(self, group, disease): subject = self.__value_or_default(email, "subject", "") timeout = self.__value_or_default(r, "timeout", 600) assignee = r["assignee"] + publish_roles = self.__value_or_default(r, "publish_roles", []) result.append(ReportConfig(r["report_name"], params, recipients, subject, timeout, - assignee)) + assignee, + publish_roles)) return result def __server(self, name): diff --git a/src/packit_client.py b/src/packit_client.py index 4905ee6..0925def 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -1,3 +1,4 @@ +import logging import requests import montagu from .packit_client_exception import PackitClientException @@ -23,10 +24,12 @@ def __get(self, relative_url, headers = None): response = requests.get(self.__url(relative_url), headers=headers) return handle_response(response) - def __post(self, relative_url, data, headers = None): - if headers is None: - headers = self.__default_headers - response requests.post(self.__url(relative_url, data=data, headers=headers) + def __post(self, relative_url, data): + response = requests.post(self.__url(relative_url, data=data, headers=self.__default_headers) + return handle_response(response) + + def __put(self, relative_url, data): + response = requests.put(self.__url(relative_url, datadata, headers=self.__default_headers) return handle_response(response) def __authenticate(self): @@ -34,9 +37,11 @@ def __authenticate(self): monty = montagu.MontaguAPI(self.__config.montagu_url, self.__config.montagu_user, config.montagu_password) packit_login_response = self.__get("/auth/login/montagu", {"Authorization": f"Bearer {monty.token}"}) + self.auth_success = True self.token = packit_login_response.token # TODO: maybe don't need to retain token as saving header? self.__default_headers = { "Authorization": f"Bearer {self.token}" } except Exception as ex: + self.auth_success = False logging.exception(ex) def __execute(self, func, *args): # TODO: maybe don't need args? @@ -50,25 +55,50 @@ def __execute(self, func, *args): # TODO: maybe don't need args? else: raise ex - def run_packet(self, packet_group, parameters): - def do_run_packet(): + def run(self, packet_group, parameters): + def do_run(): data = { "name": packet_group, "parameters": parameters, "branch": "", "hash": "" # TODO: can branch and hash be empty?? } - response = self.__post("/runner/run") + response = self.__post("/runner/run", data) return response.taskId - return __self.execute(do_run_packet) + return self.__execute(do_run) - def poll_running_packet(self, task_id): - def do_poll_running_packet(): + def poll(self, task_id): + def do_poll(): return self.__get(f"/runner/status/{task_id}") - return self.__execute(do_poll_running_packet) + return self.__execute(do_poll) + + def kill_task(self, task_id): + def do_kill_task(): + return self.__post(f"/runner/cancel/{task_id}", None) + return self.__execute(do_kill_task) + + def publish(self, packet_id, roles): + # mimic OW publishing by setting packet-level permission for a new report packet permission + # on a list of configured roles. NB: These role can either be user roles or groups. If users, + # these need to be user names not email addresses. + def do_publish_to_role(role): + data = { + "addPermissions": [ + "permission": "packit.read", + "packetId": packet_id + ], + "removePermissions": [] + } + self.__put(f"/roles/{role}/permissions", data) - def kill_running_packet(self, task_id): - def do_kill_running_packet(): - return self.__post(f"/runner/cancel/{task_id}") - return self.execute(do_kill_running_packet) \ No newline at end of file + logging.info(f"Publishing packet {name}({packet_id})") + success = True + for role in roles: + try + logging.info(f"...to role {role}") + self.__execute(do_publish_to_role(role)) + except Exception as ex: + logging.exception(ex) + success = False + return success diff --git a/src/task_run_diagnostic_reports.py b/src/task_run_diagnostic_reports.py index 6fd526e..ee2c748 100644 --- a/src/task_run_diagnostic_reports.py +++ b/src/task_run_diagnostic_reports.py @@ -12,7 +12,7 @@ from src.utils.email import send_email, Emailer from urllib.parse import quote as urlencode import logging -from src.orderlyweb_client_wrapper import OrderlyWebClientWrapper +from src.packit_client import PackitClient from src.utils.running_reports_repository import RunningReportsRepository from YTClient.YTClient import YTClient, YTException @@ -29,7 +29,7 @@ def run_diagnostic_reports(group, config = Config() reports = config.diagnostic_reports(group, disease) if len(reports) > 0: - wrapper = OrderlyWebClientWrapper(config) + packit = PackitClient(config) emailer = Emailer(config.smtp_host, config.smtp_port, config.smtp_user, config.smtp_password) yt_token = config.youtrack_token @@ -60,7 +60,7 @@ def error_callback(report, error): running_reports_repo = RunningReportsRepository(host=config.host) - return run_reports(wrapper, + return run_reports(packit, group, disease, touchstone, diff --git a/src/utils/run_reports.py b/src/utils/run_reports.py index d94b45b..7081fe2 100644 --- a/src/utils/run_reports.py +++ b/src/utils/run_reports.py @@ -1,11 +1,30 @@ import logging import time - - -def publish_report(wrapper, name, version): +from enum import Enum + +# TODO: what do these actually mean!? +class TaskStatus(Enum): + PENDING = "PENDING" + RUNNING = "RUNNING" + COMPLETE = "COMPLETE" + ERROR = "ERROR" + CANCELLED = "CANCELLED" + DIED = "DIED" + TIMEOUT = "TIMEOUT" + IMPOSSIBLE = "IMPOSSIBLE" + DEFERRED = "DEFERRED" + MOVED = "MOVED" + +def task_is_finished(poll_response): + return poll_response.status not in [ + TaskStatus.PENDING, + TaskStatus.RUNNING + ] + +def publish_report(wrapper, name, version, roles): try: - logging.info("Publishing report version {}-{}".format(name, version)) - return wrapper.execute(wrapper.ow.publish_report, name, version) + logging.info(f"Publishing packet {name}({version})") + return packit.publish(version, roles) except Exception as ex: logging.exception(ex) return False @@ -14,14 +33,13 @@ def publish_report(wrapper, name, version): def params_to_string(params): return ", ".join([f"{key}={value}" for key, value in params.items()]) - -def run_reports(wrapper, group, disease, touchstone, config, reports, +def run_reports(packit, group, disease, touchstone, config, reports, success_callback, error_callback, running_reports_repo): running_reports = {} new_versions = {} - if wrapper.ow is None: - error = "Orderlyweb authentication failed; could not begin task" + if not packit.auth_success: + error = "Packit authentication failed; could not begin task" for report in reports: error_callback(report, error) logging.error(error) @@ -29,13 +47,13 @@ def run_reports(wrapper, group, disease, touchstone, config, reports, # Start configured reports for report in reports: - # Kill any currently running report for this group/disease/report + # Kill any currently running task for this group/disease/report already_running = running_reports_repo.get(group, disease, report.name) if already_running is not None: try: - logging.info("Killing already running report: {}. Key is {}" + logging.info("Killing already running task: {}. Key is {}" .format(report.name, already_running)) - wrapper.execute(wrapper.ow.kill_report, already_running) + packit.kill_task(already_running) except Exception as ex: logging.exception(ex) @@ -45,10 +63,15 @@ def run_reports(wrapper, group, disease, touchstone, config, reports, parameters["touchstone_name"] = touchstone.rsplit('-', 1)[0] try: - key = wrapper.execute(wrapper.ow.run_report, - report.name, - parameters, - report.timeout) + key = packit.run( + report.name, + parameters + ) + # TODO: how was timeout used? + #key = wrapper.execute(wrapper.ow.run_report, + # report.name, + # parameters, + # report.timeout) running_reports[key] = report # Save key to shared data - may be killed by subsequent task @@ -61,7 +84,7 @@ def run_reports(wrapper, group, disease, touchstone, config, reports, error_callback(report, str(ex)) logging.exception(ex) - # Poll running reports until they complete + # Poll running tasks until they complete report_poll_seconds = config.report_poll_seconds while len(running_reports.items()) > 0: finished = [] @@ -69,26 +92,29 @@ def run_reports(wrapper, group, disease, touchstone, config, reports, for key in keys: report = running_reports[key] try: - result = wrapper.execute(wrapper.ow.report_status, key) - if result.finished: + result = packit.poll(key) + if task_is_finished(result): finished.append(key) - if result.success: - logging.info("Success for key {}. New version is {}" - .format(key, result.version)) + if result.status == TaskStatus.COMPLETE: + logging.info("Success for key {}. New packet id is {}" + .format(key, result.packetId)) - version = result.version + version = result.packetId name = report.name - published = publish_report(wrapper, name, version) - if published: - logging.info( - "Successfully published report version {}-{}" - .format(name, version)) - success_callback(report, version) - else: - error = "Failed to publish report version {}-{}"\ - .format(name, version) - logging.error(error) - error_callback(report, error) + + report_config = filter(lambda: report: report.name == name, reports) + if len(report_config) > 0 and len(report_config[0].publish_roles > 0): + published = publish_report(wrapper, name, version, report_config[0].publish_roles) + if published: + logging.info( + "Successfully published report version {}-{}" + .format(name, version)) + success_callback(report, version) + else: + error = "Failed to publish report version {}-{}"\ + .format(name, version) + logging.error(error) + error_callback(report, error) new_versions[version] = { "published": published, "report": name @@ -98,7 +124,7 @@ def run_reports(wrapper, group, disease, touchstone, config, reports, .format(key, result.status) logging.error(error) # don't invoke error callback for cancelled runs - if result.status != "interrupted": + if result.status != TaskStatus.CANCELLED: error_callback(report, error) except Exception as ex: From 1a6c7df12d45487e11886121d00483260b43442a Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 7 Jul 2025 16:50:23 +0100 Subject: [PATCH 05/36] remove timeout - not supported by new runner --- src/config.py | 4 ---- src/utils/run_reports.py | 8 +------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/config.py b/src/config.py index 946052a..3fe2433 100644 --- a/src/config.py +++ b/src/config.py @@ -8,14 +8,12 @@ def __init__(self, parameters, success_email_recipients, success_email_subject, - timeout, assignee, publish_roles): self.name = name self.parameters = parameters self.success_email_recipients = success_email_recipients self.success_email_subject = success_email_subject - self.timeout = timeout self.assignee = assignee self.publish_roles = publish_roles @@ -98,7 +96,6 @@ def diagnostic_reports(self, group, disease): email = self.__value_or_default(r, "success_email", {}) recipients = self.__value_or_default(email, "recipients", []) subject = self.__value_or_default(email, "subject", "") - timeout = self.__value_or_default(r, "timeout", 600) assignee = r["assignee"] publish_roles = self.__value_or_default(r, "publish_roles", []) @@ -106,7 +103,6 @@ def diagnostic_reports(self, group, disease): params, recipients, subject, - timeout, assignee, publish_roles)) return result diff --git a/src/utils/run_reports.py b/src/utils/run_reports.py index 7081fe2..b1ed761 100644 --- a/src/utils/run_reports.py +++ b/src/utils/run_reports.py @@ -67,19 +67,13 @@ def run_reports(packit, group, disease, touchstone, config, reports, report.name, parameters ) - # TODO: how was timeout used? - #key = wrapper.execute(wrapper.ow.run_report, - # report.name, - # parameters, - # report.timeout) running_reports[key] = report # Save key to shared data - may be killed by subsequent task running_reports_repo.set(group, disease, report.name, key) logging.info("Running report: {} with parameters {}. Key is {}. " - "Timeout is {}s." .format(report.name, params_to_string(parameters), - key, report.timeout)) + key)) except Exception as ex: error_callback(report, str(ex)) logging.exception(ex) From 5b4c6e1ae5a1eb1dd167aa3f06c789ae7cf3f113 Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 7 Jul 2025 17:07:42 +0100 Subject: [PATCH 06/36] replace most references to "version" with "packet_id" --- config/config.yml | 4 ++-- src/config.py | 4 ++-- src/packit_client.py | 1 - src/task_run_diagnostic_reports.py | 16 ++++++++-------- src/utils/run_reports.py | 27 ++++++++++++--------------- 5 files changed, 24 insertions(+), 28 deletions(-) diff --git a/config/config.yml b/config/config.yml index ba70f9c..52ac46a 100644 --- a/config/config.yml +++ b/config/config.yml @@ -4,8 +4,8 @@ servers: url: http://localhost:8080 user: test.user@example.com password: password - packit: - url: http://localhost:3000 + packit_api: + url: http://localhost:8081 # TODO: expose port in run deps youtrack: token: None smtp: diff --git a/src/config.py b/src/config.py index 3fe2433..19cf0c0 100644 --- a/src/config.py +++ b/src/config.py @@ -46,8 +46,8 @@ def montagu_password(self): return self.__montagu()["password"] @property - def orderlyweb_url(self): - return self.__server("orderlyweb")["url"] + def packit_api_url(self): + return self.__server("packit_api")["url"] @property def youtrack_token(self): diff --git a/src/packit_client.py b/src/packit_client.py index 0925def..31e7e0d 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -9,7 +9,6 @@ def __init__(self, config): self.__authenticate() def __url(self, relative_url): - # TODO: add base url to config return f"{self.__config.packit_api_url}{relative_url}" @staticmethod diff --git a/src/task_run_diagnostic_reports.py b/src/task_run_diagnostic_reports.py index ee2c748..fe4f276 100644 --- a/src/task_run_diagnostic_reports.py +++ b/src/task_run_diagnostic_reports.py @@ -40,10 +40,10 @@ def run_diagnostic_reports(group, yt = YTClient('https://mrc-ide.myjetbrains.com/youtrack/', token=yt_token) - def success_callback(report, version): + def success_callback(report, packet_id): send_diagnostic_report_email(emailer, report, - version, + packet_id, group, disease, touchstone, @@ -52,7 +52,7 @@ def success_callback(report, version): config, *additional_recipients) create_ticket(group, disease, touchstone, scenario, - report, version, None, yt, config) + report, packet_id, None, yt, config) def error_callback(report, error): create_ticket(group, disease, touchstone, scenario, @@ -76,16 +76,16 @@ def error_callback(report, error): def create_ticket(group, disease, touchstone, scenario, - report: ReportConfig, version, + report: ReportConfig, packet_id, error, yt: YTClient, config: Config): try: - report_success = version is not None + report_success = packet_id is not None summary = "Check & share diag report with {} ({}) {}" if \ report_success else \ "Run, check & share diag report with {} ({}) {}" - result = get_version_url(report, version, config) if \ + result = get_version_url(report, packet_id, config) if \ report_success else \ "Auto-run failed with error: {}".format(error) description = "Report run triggered by upload to scenario: {}. {}"\ @@ -130,7 +130,7 @@ def create_tags(yt, group, disease, touchstone, report): def send_diagnostic_report_email(emailer, report, - version, + packet_id, group, disease, touchstone, @@ -139,7 +139,7 @@ def send_diagnostic_report_email(emailer, config, *additional_recipients): template_values = { - "report_version_url": get_version_url(report, version, config), + "report_version_url": get_version_url(report, packet_id, config), "disease": disease, "group": group, "touchstone": touchstone, diff --git a/src/utils/run_reports.py b/src/utils/run_reports.py index b1ed761..b49d601 100644 --- a/src/utils/run_reports.py +++ b/src/utils/run_reports.py @@ -2,7 +2,6 @@ import time from enum import Enum -# TODO: what do these actually mean!? class TaskStatus(Enum): PENDING = "PENDING" RUNNING = "RUNNING" @@ -21,10 +20,10 @@ def task_is_finished(poll_response): TaskStatus.RUNNING ] -def publish_report(wrapper, name, version, roles): +def publish_report(wrapper, name, packet_id, roles): try: - logging.info(f"Publishing packet {name}({version})") - return packit.publish(version, roles) + logging.info(f"Publishing report packet {name}({packet_id})") + return packit.publish(packet_id, roles) except Exception as ex: logging.exception(ex) return False @@ -36,14 +35,14 @@ def params_to_string(params): def run_reports(packit, group, disease, touchstone, config, reports, success_callback, error_callback, running_reports_repo): running_reports = {} - new_versions = {} + new_packets = {} if not packit.auth_success: error = "Packit authentication failed; could not begin task" for report in reports: error_callback(report, error) logging.error(error) - return new_versions + return new_packets # Start configured reports for report in reports: @@ -93,23 +92,21 @@ def run_reports(packit, group, disease, touchstone, config, reports, logging.info("Success for key {}. New packet id is {}" .format(key, result.packetId)) - version = result.packetId + packet_id = result.packetId name = report.name report_config = filter(lambda: report: report.name == name, reports) if len(report_config) > 0 and len(report_config[0].publish_roles > 0): - published = publish_report(wrapper, name, version, report_config[0].publish_roles) + published = publish_report(wrapper, name, packet_id, report_config[0].publish_roles) if published: logging.info( - "Successfully published report version {}-{}" - .format(name, version)) - success_callback(report, version) + f"Successfully published report packet {name} ({packet_id})") + success_callback(report, packet_id) else: - error = "Failed to publish report version {}-{}"\ - .format(name, version) + error = f"Failed to publish report packet {name} ({packet_id})") logging.error(error) error_callback(report, error) - new_versions[version] = { + new_packets[packet_id] = { "published": published, "report": name } @@ -135,4 +132,4 @@ def run_reports(packit, group, disease, touchstone, config, reports, key) time.sleep(report_poll_seconds) - return new_versions + return new_packet From 4704cc4ed2c50e474ec04f1ab8c955bf7f16b3b4 Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 8 Jul 2025 16:26:27 +0100 Subject: [PATCH 07/36] replace OW with packit in deps --- docker-compose.yml | 10 ++++++- scripts/clear-docker.sh | 4 ++- scripts/packit.yml | 48 ++++++++++++++++++++++++++++++++- scripts/run-dependencies.sh | 54 +++++++++++++++++++++++++++++-------- 4 files changed, 102 insertions(+), 14 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 484fcfe..cacd67c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -23,8 +23,16 @@ services: ports: - "5432:5432" command: /etc/montagu/postgresql.test.conf + contrib: + image: ${REGISTRY}/montagu-contrib-portal:master + depends_on: + - api + admin: + image: ${REGISTRY}/montagu-admin-portal:master + depends_on: + - api smtp_server: image: reachfive/fake-smtp-server ports: - "1025:1025" - - "1080:1080" \ No newline at end of file + - "1080:1080" diff --git a/scripts/clear-docker.sh b/scripts/clear-docker.sh index a210d3e..6d9c1a1 100755 --- a/scripts/clear-docker.sh +++ b/scripts/clear-docker.sh @@ -1,4 +1,6 @@ +set -x + docker stop $(docker ps -aq) docker rm $(docker ps -aq) docker network prune --force -docker volume prune --force \ No newline at end of file +docker volume prune --force diff --git a/scripts/packit.yml b/scripts/packit.yml index 67769e0..d4e32aa 100644 --- a/scripts/packit.yml +++ b/scripts/packit.yml @@ -1 +1,47 @@ -# TODO: this should largely match uat/basicauth configs - but needs runner! \ No newline at end of file +container_prefix: montagu + +protect_data: false + +repo: ghcr.io/mrc-ide + +network: montagu_default + +volumes: + outpack: montagu_outpack_volume + packit_db: montagu_packit_db + orderly_library: montagu_orderly_library + orderly_logs: montagu_orderly_logs + +outpack: + server: + name: outpack_server + tag: main + +packit: + base_url: https://localhost + api: + name: packit-api + tag: main + app: + name: montagu-packit + tag: main + db: + name: packit-db + tag: main + user: packituser + password: changeme + auth: + enabled: true + auth_method: preauth + # We'll get this from the vault on production + jwt: + secret: "0b4g4f8z4mdsrhoxfde2mam8f00vmt0f" + expiry_days: 1 + +orderly-runner: + image: + name: orderly.runner + tag: main + git: + url: https://github.com/reside-ic/orderly2-example.git + workers: 1 diff --git a/scripts/run-dependencies.sh b/scripts/run-dependencies.sh index 18011a4..e7305f5 100755 --- a/scripts/run-dependencies.sh +++ b/scripts/run-dependencies.sh @@ -1,14 +1,15 @@ #!/usr/bin/env bash set -ex -here=$(dirname $0) +export REGISTRY=vimc +here=$(dirname $0) ./scripts/clear-docker.sh docker network prune -f -export REGISTRY=vimc export NETWORK=montagu_default + # Run the API and database docker compose pull docker compose --project-name montagu up -d @@ -16,11 +17,6 @@ docker compose --project-name montagu up -d # Clear redis docker exec montagu-mq-1 redis-cli FLUSHALL -# Install packit -pip3 install constellation -pip3 install packit-deploy -packit start --pull $here - # Start the APIs docker exec montagu-api-1 mkdir -p /etc/montagu/api/ docker exec montagu-api-1 touch /etc/montagu/api/go_signal @@ -40,7 +36,43 @@ $here/montagu_cli.sh add "Test User" test.user \ $here/montagu_cli.sh addRole test.user user -# TODO: Add user to Packit -# Add user to orderlyweb -#$here/orderlyweb_cli.sh add-users test.user@example.com -#$here/orderlyweb_cli.sh grant test.user@example.com */reports.read */reports.run */reports.review +# Run packit +hatch env run pip3 install constellation +hatch env run pip3 install packit-deploy +# TODO: For some reason packit is emitting exit code 1 despite apparently succeeding. Allow this for now... +set +e +hatch env run -- packit start --pull $here +echo Packit deployed with exit code $? +set -e + +# Run the proxy here, not through docker compose - it needs packit to be running before it will start up +MONTAGU_PROXY_TAG=vimc/montagu-reverse-proxy:master +docker pull $MONTAGU_PROXY_TAG +docker run -d \ + -p "443:443" -p "80:80" \ + --name reverse-proxy \ + --network montagu_default\ + $MONTAGU_PROXY_TAG 443 localhost + +# Add user to packit, as admin +USERNAME='test.user' +EMAIL='test.user@example.com' +DISPLAY_NAME='Test User' +ROLE='ADMIN' +docker exec montagu-packit-db create-preauth-user --username "$USERNAME" --email "$EMAIL" --displayname "$DISPLAY_NAME" --role "$ROLE" + +# TODO: Get packets into packit... + +# From now on, if the user presses Ctrl+C we should teardown gracefully +function cleanup() { + docker compose down -v + docker container stop reverse-proxy + docker container rm reverse-proxy -v + # TODO: This requires user interaction - but clear-docker does not remove volumes! + hatch env run -- packit stop ./scripts --network --volume +} +trap cleanup EXIT + +# Wait for Ctrl+C +echo "Ready to use. Press Ctrl+C to teardown." +sleep infinity From b42f3472b2afc3ff904ad2d3d3aa160647361c9e Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 8 Jul 2025 17:52:38 +0100 Subject: [PATCH 08/36] small deps tweaks and syntax fixes --- README.md | 5 +++-- config/docker_config.yml | 6 +++-- docker-compose.yml | 1 - scripts/packit.yml | 3 +++ scripts/run-dependencies.sh | 3 +-- src/packit_client.py | 44 ++++++++++++++++++------------------- src/utils/run_reports.py | 7 +++--- 7 files changed, 37 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 0f36444..0f6f2a0 100644 --- a/README.md +++ b/README.md @@ -13,12 +13,13 @@ execute tasks, coordinated by Celery. Celery supports various message brokers in Clone the repo anywhere and install dependencies with (from the repo root): ``` -pip3 install --user -r requirements.txt +hatch shell +pip3 install -r requirements.txt ``` ## Development -Run dependencies (Montagu API and DB, OrderlyWeb and a local Redis message queue in docker) with `scripts/run-dependencies.sh` +Run dependencies (Montagu API and DB, Proxy, Packit and a local Redis message queue in docker) with `scripts/run-dependencies.sh` Dependencies also include a fake smtp server run from a [docker image](https://hub.docker.com/r/reachfive/fake-smtp-server) to enable development and testing of email functionality. You can see a web front end for the emails 'sent' via this server diff --git a/config/docker_config.yml b/config/docker_config.yml index 2688eae..2cea83d 100644 --- a/config/docker_config.yml +++ b/config/docker_config.yml @@ -19,7 +19,7 @@ tasks: reports: testGroup: testDisease: - - report_name: diagnostic + - report_name: explicit success_email: recipients: - minimal_modeller@example.com @@ -28,7 +28,9 @@ tasks: assignee: a.hill - report_name: diagnostic-param parameters: - nmin: 0 + - a: 1 + - b: 2 + - c: 3 success_email: recipients: - other_modeller@example.com diff --git a/docker-compose.yml b/docker-compose.yml index cacd67c..6f688c9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,4 +1,3 @@ -version: '3' services: mq: image: redis diff --git a/scripts/packit.yml b/scripts/packit.yml index d4e32aa..0e01a14 100644 --- a/scripts/packit.yml +++ b/scripts/packit.yml @@ -16,6 +16,9 @@ outpack: server: name: outpack_server tag: main + migrate: + name: outpack.orderly + tag: main packit: base_url: https://localhost diff --git a/scripts/run-dependencies.sh b/scripts/run-dependencies.sh index e7305f5..f2d46aa 100755 --- a/scripts/run-dependencies.sh +++ b/scripts/run-dependencies.sh @@ -55,14 +55,13 @@ docker run -d \ $MONTAGU_PROXY_TAG 443 localhost # Add user to packit, as admin +# TODO: This is flaky, sadly. Need to wait until packit-db is ready? USERNAME='test.user' EMAIL='test.user@example.com' DISPLAY_NAME='Test User' ROLE='ADMIN' docker exec montagu-packit-db create-preauth-user --username "$USERNAME" --email "$EMAIL" --displayname "$DISPLAY_NAME" --role "$ROLE" -# TODO: Get packets into packit... - # From now on, if the user presses Ctrl+C we should teardown gracefully function cleanup() { docker compose down -v diff --git a/src/packit_client.py b/src/packit_client.py index 31e7e0d..521733d 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -24,11 +24,11 @@ def __get(self, relative_url, headers = None): return handle_response(response) def __post(self, relative_url, data): - response = requests.post(self.__url(relative_url, data=data, headers=self.__default_headers) + response = requests.post(self.__url(relative_url), data=data, headers=self.__default_headers) return handle_response(response) def __put(self, relative_url, data): - response = requests.put(self.__url(relative_url, datadata, headers=self.__default_headers) + response = requests.put(self.__url(relative_url), datadata, headers=self.__default_headers) return handle_response(response) def __authenticate(self): @@ -77,27 +77,27 @@ def do_kill_task(): return self.__post(f"/runner/cancel/{task_id}", None) return self.__execute(do_kill_task) - def publish(self, packet_id, roles): - # mimic OW publishing by setting packet-level permission for a new report packet permission - # on a list of configured roles. NB: These role can either be user roles or groups. If users, - # these need to be user names not email addresses. - def do_publish_to_role(role): - data = { - "addPermissions": [ + def publish(self, packet_id, roles): + # mimic OW publishing by setting packet-level permission for a new report packet permission + # on a list of configured roles. NB: These role can either be user roles or groups. If users, + # these need to be user names not email addresses. + def do_publish_to_role(role): + data = { + "addPermissions": [{ "permission": "packit.read", "packetId": packet_id - ], + }], "removePermissions": [] - } - self.__put(f"/roles/{role}/permissions", data) + } + self.__put(f"/roles/{role}/permissions", data) - logging.info(f"Publishing packet {name}({packet_id})") - success = True - for role in roles: - try - logging.info(f"...to role {role}") - self.__execute(do_publish_to_role(role)) - except Exception as ex: - logging.exception(ex) - success = False - return success + logging.info(f"Publishing packet {name}({packet_id})") + success = True + for role in roles: + try: + logging.info(f"...to role {role}") + self.__execute(do_publish_to_role(role)) + except Exception as ex: + logging.exception(ex) + success = False + return success diff --git a/src/utils/run_reports.py b/src/utils/run_reports.py index b49d601..a50342a 100644 --- a/src/utils/run_reports.py +++ b/src/utils/run_reports.py @@ -95,15 +95,16 @@ def run_reports(packit, group, disease, touchstone, config, reports, packet_id = result.packetId name = report.name - report_config = filter(lambda: report: report.name == name, reports) + report_config = filter(lambda report: report.name == name, reports) if len(report_config) > 0 and len(report_config[0].publish_roles > 0): published = publish_report(wrapper, name, packet_id, report_config[0].publish_roles) if published: logging.info( - f"Successfully published report packet {name} ({packet_id})") + f"Successfully published report packet {name} ({packet_id})" + ) success_callback(report, packet_id) else: - error = f"Failed to publish report packet {name} ({packet_id})") + error = f"Failed to publish report packet {name} ({packet_id})" logging.error(error) error_callback(report, error) new_packets[packet_id] = { From fa89f7f914ff7b7e96503c386f00be17c5d39248 Mon Sep 17 00:00:00 2001 From: Emma Date: Thu, 10 Jul 2025 16:11:38 +0100 Subject: [PATCH 09/36] got a task running successfully, but messily --- config/config.yml | 9 +++-- config/docker_config.yml | 13 +++++-- scripts/clear-docker.sh | 2 +- scripts/run-dependencies.sh | 18 ++++++++- src/config.py | 8 +++- src/packit_client.py | 38 +++++++++++-------- src/packit_client_exception.py | 4 +- src/task_run_diagnostic_reports.py | 12 +++--- src/utils/run_reports.py | 33 ++++++++-------- .../test_task_run_diagnostic_reports.py | 14 +++---- 10 files changed, 93 insertions(+), 58 deletions(-) diff --git a/config/config.yml b/config/config.yml index 52ac46a..9a0a4e4 100644 --- a/config/config.yml +++ b/config/config.yml @@ -4,8 +4,9 @@ servers: url: http://localhost:8080 user: test.user@example.com password: password - packit_api: - url: http://localhost:8081 # TODO: expose port in run deps + packit: + url: https://localhost/packit + disable_certificate_verify: true youtrack: token: None smtp: @@ -34,7 +35,9 @@ tasks: - Funders - report_name: diagnostic-param parameters: - nmin: 0 + a: 1 + b: 2 + c: 3 success_email: recipients: - other_modeller@example.com diff --git a/config/docker_config.yml b/config/docker_config.yml index 2cea83d..63259b8 100644 --- a/config/docker_config.yml +++ b/config/docker_config.yml @@ -19,23 +19,28 @@ tasks: reports: testGroup: testDisease: - - report_name: explicit + - report_name: diagnostic success_email: recipients: - minimal_modeller@example.com - science@example.com subject: "VIMC diagnostic report: {touchstone} - {group} - {disease}" assignee: a.hill + publish_roles: + - minimal.modeller + - Funders - report_name: diagnostic-param parameters: - - a: 1 - - b: 2 - - c: 3 + a: 1 + b: 2 + c: 3 success_email: recipients: - other_modeller@example.com - science@example.com subject: "New version of another Orderly report" assignee: e.russell + publish_roles: + - other.modeller archive_folder_contents: min_file_age_seconds: 0 diff --git a/scripts/clear-docker.sh b/scripts/clear-docker.sh index 6d9c1a1..811b996 100755 --- a/scripts/clear-docker.sh +++ b/scripts/clear-docker.sh @@ -3,4 +3,4 @@ set -x docker stop $(docker ps -aq) docker rm $(docker ps -aq) docker network prune --force -docker volume prune --force +docker volume prune --force \ No newline at end of file diff --git a/scripts/run-dependencies.sh b/scripts/run-dependencies.sh index f2d46aa..453c2bc 100755 --- a/scripts/run-dependencies.sh +++ b/scripts/run-dependencies.sh @@ -45,6 +45,8 @@ hatch env run -- packit start --pull $here echo Packit deployed with exit code $? set -e +docker exec montagu-packit-db wait-for-db + # Run the proxy here, not through docker compose - it needs packit to be running before it will start up MONTAGU_PROXY_TAG=vimc/montagu-reverse-proxy:master docker pull $MONTAGU_PROXY_TAG @@ -54,6 +56,17 @@ docker run -d \ --network montagu_default\ $MONTAGU_PROXY_TAG 443 localhost +# give packit api some time to migrate the db... +sleep 10 + +# create roles to publish to... +docker exec -i montagu-packit-db psql -U packituser -d packit --single-transaction < 0 and len(report_config[0].publish_roles > 0): - published = publish_report(wrapper, name, packet_id, report_config[0].publish_roles) + report_config = next(filter(lambda report: report.name == name, reports), None) + if report_config is not None: + time.sleep(11) # TODO: something better! need to wait for packit to be imported => Detail: Permission not found + published = publish_report(packit, name, packet_id, report_config.publish_roles) if published: logging.info( f"Successfully published report packet {name} ({packet_id})" ) success_callback(report, packet_id) else: + #raise Exception(f"RESULT: {result}") error = f"Failed to publish report packet {name} ({packet_id})" logging.error(error) error_callback(report, error) @@ -112,11 +114,12 @@ def run_reports(packit, group, disease, touchstone, config, reports, "report": name } else: + raise Exception(f"ERROR RESULT {result}") error = "Failure for key {}. Status: {}"\ - .format(key, result.status) + .format(key, result["status"]) logging.error(error) # don't invoke error callback for cancelled runs - if result.status != TaskStatus.CANCELLED: + if result["status"] != TaskStatus.CANCELLED: error_callback(report, error) except Exception as ex: @@ -133,4 +136,4 @@ def run_reports(packit, group, disease, touchstone, config, reports, key) time.sleep(report_poll_seconds) - return new_packet + return new_packets diff --git a/test/integration/test_task_run_diagnostic_reports.py b/test/integration/test_task_run_diagnostic_reports.py index f033759..4c4a2f4 100644 --- a/test/integration/test_task_run_diagnostic_reports.py +++ b/test/integration/test_task_run_diagnostic_reports.py @@ -103,15 +103,15 @@ def test_run_diagnostic_reports(): diagnostic_is_first = result[versions[0]]["report"] == "diagnostic" if diagnostic_is_first: - report_1 = "diagnostic" - report_2 = "diagnostic-param" - email_props = [diagnostic_email_props, diagnostic_param_email_props] + report_1 = "diagnostic" + report_2 = "diagnostic-param" + email_props = [diagnostic_email_props, diagnostic_param_email_props] else: - report_1 = "diagnostic-param" - report_2 = "diagnostic" - email_props = [diagnostic_param_email_props, diagnostic_email_props] + report_1 = "diagnostic-param" + report_2 = "diagnostic" + email_props = [diagnostic_param_email_props, diagnostic_email_props] - url_template = "http://localhost:8888/report/{}/{}/" + url_template = "https://localhost/packit/{}/{}/" url_1 = url_template.format(report_1, versions[0]) url_2 = url_template.format(report_2, versions[1]) From cb48cbedbf8110f070f391c5b031e877d9c09e87 Mon Sep 17 00:00:00 2001 From: Emma Date: Fri, 11 Jul 2025 13:59:58 +0100 Subject: [PATCH 10/36] specify commit when run report --- scripts/run-dependencies.sh | 18 +++++++++--------- src/packit_client.py | 18 +++++++++++++++--- src/utils/run_reports.py | 29 +++++++++-------------------- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/scripts/run-dependencies.sh b/scripts/run-dependencies.sh index 453c2bc..e13d7e4 100755 --- a/scripts/run-dependencies.sh +++ b/scripts/run-dependencies.sh @@ -9,7 +9,6 @@ docker network prune -f export NETWORK=montagu_default - # Run the API and database docker compose pull docker compose --project-name montagu up -d @@ -39,7 +38,7 @@ $here/montagu_cli.sh addRole test.user user # Run packit hatch env run pip3 install constellation hatch env run pip3 install packit-deploy -# TODO: For some reason packit is emitting exit code 1 despite apparently succeeding. Allow this for now... +# For some reason packit is emitting exit code 1 despite apparently succeeding. Allow this for now... set +e hatch env run -- packit start --pull $here echo Packit deployed with exit code $? @@ -57,7 +56,7 @@ docker run -d \ $MONTAGU_PROXY_TAG 443 localhost # give packit api some time to migrate the db... -sleep 10 +sleep 5 # create roles to publish to... docker exec -i montagu-packit-db psql -U packituser -d packit --single-transaction < Detail: Permission not found published = publish_report(packit, name, packet_id, report_config.publish_roles) if published: logging.info( @@ -105,7 +96,6 @@ def run_reports(packit, group, disease, touchstone, config, reports, ) success_callback(report, packet_id) else: - #raise Exception(f"RESULT: {result}") error = f"Failed to publish report packet {name} ({packet_id})" logging.error(error) error_callback(report, error) @@ -114,12 +104,11 @@ def run_reports(packit, group, disease, touchstone, config, reports, "report": name } else: - raise Exception(f"ERROR RESULT {result}") error = "Failure for key {}. Status: {}"\ .format(key, result["status"]) logging.error(error) # don't invoke error callback for cancelled runs - if result["status"] != TaskStatus.CANCELLED: + if result["status"] != TASK_STATUS_CANCELLED: error_callback(report, error) except Exception as ex: From 0aa75e96ee333602989467fabfdc291b78dbfca7 Mon Sep 17 00:00:00 2001 From: Emma Date: Fri, 11 Jul 2025 16:14:41 +0100 Subject: [PATCH 11/36] poll for packet before publish --- src/packit_client.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/packit_client.py b/src/packit_client.py index 8201204..d4169d2 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -68,6 +68,23 @@ def __get_latest_commit_for_branch(self, branch): raise Exception(f"Git details not found for branch {branch}") return branch_details["commitHash"] + def __wait_for_packet_to_be_imported(self, packet_id): + # packet api imports new packets every 10s - poll it for a generous + # 30s to find a new packet before attempting to publish it + poll_seconds = 2 + seconds_max = 30 + poll_max = seconds_max / poll_seconds + poll_counter = 0 + while poll_counter <= poll_max: + try: + self.__get(f"/packets/{packet_id}") + return + except PackitClientException as ex: + logging.info(f"Waiting for packet {packet_id}...") + poll_counter = poll_counter + 1 + time.sleep(poll_seconds) + raise Exception(f"Packet {packet_id} was not imported into Packit after {seconds_max}s") + def run(self, packet_group, parameters): def do_run(): @@ -94,6 +111,7 @@ def do_kill_task(): return self.__post(f"/runner/cancel/{task_id}", None) return self.__execute(do_kill_task) + def publish(self, name, packet_id, roles): # mimic OW publishing by setting packet-level permission for a new report packet permission # on a list of configured roles. NB: These role can either be user roles or groups. If users, @@ -108,7 +126,8 @@ def do_publish_to_role(role): } self.__put(f"/roles/{role}/permissions", data) - time.sleep(11) # TODO: :( We need to wait for scheduled packet import + self.__wait_for_packet_to_be_imported(packet_id) + logging.info(f"Publishing packet {name}({packet_id})") success = True for role in roles: From d52cb05350831dc91104a4201008770b31399ad1 Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 14 Jul 2025 14:39:23 +0100 Subject: [PATCH 12/36] packit client unit tests --- requirements.txt | 1 + src/packit_client.py | 2 +- src/task_run_diagnostic_reports.py | 2 - test/unit/test_packit_client.py | 64 ++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 test/unit/test_packit_client.py diff --git a/requirements.txt b/requirements.txt index db26768..e0ba857 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,3 +5,4 @@ montagu>=0.0.2 orderlyweb-api>=1.0.0 git+https://github.com/reside-ic/youtrack-rest-python-library constellation +requests-mock diff --git a/src/packit_client.py b/src/packit_client.py index d4169d2..6fa233f 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -40,7 +40,7 @@ def __authenticate(self): self.__config.montagu_password) packit_login_response = self.__get("/auth/login/montagu", {"Authorization": f"Bearer {monty.token}"}) self.auth_success = True - self.token = packit_login_response["token"] # TODO: maybe don't need to retain token as saving header? + self.token = packit_login_response["token"] self.__default_headers = { "Authorization": f"Bearer {self.token}", "Content-Type": f"application/json" diff --git a/src/task_run_diagnostic_reports.py b/src/task_run_diagnostic_reports.py index 04da267..bbfdcbe 100644 --- a/src/task_run_diagnostic_reports.py +++ b/src/task_run_diagnostic_reports.py @@ -80,8 +80,6 @@ def create_ticket(group, disease, touchstone, scenario, error, yt: YTClient, config: Config): - # TODO: reinstate! - return try: report_success = packet_id is not None summary = "Check & share diag report with {} ({}) {}" if \ diff --git a/test/unit/test_packit_client.py b/test/unit/test_packit_client.py new file mode 100644 index 0000000..710731c --- /dev/null +++ b/test/unit/test_packit_client.py @@ -0,0 +1,64 @@ +import json +import montagu +import requests_mock +from src.packit_client import PackitClient +from unittest.mock import patch, MagicMock + +PACKIT_URL = "http://test-packit" + +class MockConfig: + def __init__(self): + self.disable_certificate_verify = False + self.montagu_url = "http://test-montagu" + self.montagu_user = "test.montagu.user" + self.montagu_password = "montagu_password" + self.packit_url = PACKIT_URL + + +config = MockConfig() + +def mock_auth(mock_montagu_api_class, requests_mock): + mock_montagu_api = mock_montagu_api_class.return_value + mock_montagu_api.token = "test-montagu-token" + requests_mock.get(f"{PACKIT_URL}/api/auth/login/montagu", text = json.dumps({"token": "test-packit-token"} )) + +@patch("montagu.MontaguAPI") +def test_authenticates_on_init(MockMontaguAPI, requests_mock): + mock_auth(MockMontaguAPI, requests_mock) + + sut = PackitClient(config) + + MockMontaguAPI.assert_called_with("http://test-montagu", "test.montagu.user", "montagu_password") + auth_call = requests_mock.request_history[0] + assert auth_call.method == "GET" + assert auth_call.url == f"{PACKIT_URL}/api/auth/login/montagu" + assert auth_call.headers["Authorization"] == "Bearer test-montagu-token" + assert sut.auth_success + assert sut.token == "test-packit-token" + + +#@patch("montagu.MontaguAPI") +#def test_run(): +# mock_auth(MockMontaguAPI, requests_mock) + +#def test_reauthenticates_on_401 + +#def test_raises_exception_on_unexpected_status + +#def test_poll_status + +@patch("montagu.MontaguAPI") +def test_kill_task(MockMontaguAPI, requests_mock): + mock_auth(MockMontaguAPI, requests_mock) + mock_kill_response = { "status": "dead" } + requests_mock.post(f"{PACKIT_URL}/api/runner/cancel/test-task-id", text = json.dumps(mock_kill_response)) + + sut = PackitClient(config) + resp = sut.kill_task("test-task-id") + + assert resp == mock_kill_response + req = requests_mock.request_history[1] + assert req.headers["Authorization"] == "Bearer test-packit-token" + +#def test_publish + From 750de03eb16983fee570d45f2c8454c4f363bffa Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 14 Jul 2025 16:49:25 +0100 Subject: [PATCH 13/36] more unit tests --- src/packit_client.py | 8 +++- test/unit/test_packit_client.py | 79 +++++++++++++++++++++++++++++---- 2 files changed, 76 insertions(+), 11 deletions(-) diff --git a/src/packit_client.py b/src/packit_client.py index 6fa233f..4fce56d 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -20,6 +20,10 @@ def handle_response(response): raise PackitClientException(response) return response.json() + @staticmethod + def serialize(data): + return None if data is None else json.dumps(data) + def __get(self, relative_url, headers = None): if headers is None: headers = self.__default_headers @@ -27,11 +31,11 @@ def __get(self, relative_url, headers = None): return PackitClient.handle_response(response) def __post(self, relative_url, data): - response = requests.post(self.__url(relative_url), data=json.dumps(data), headers=self.__default_headers, verify = self.__verify) + response = requests.post(self.__url(relative_url), data=PackitClient.serialize(data), headers=self.__default_headers, verify = self.__verify) return PackitClient.handle_response(response) def __put(self, relative_url, data): - response = requests.put(self.__url(relative_url), data=json.dumps(data), headers=self.__default_headers, verify = self.__verify) + response = requests.put(self.__url(relative_url), data=PackitClient.serialize(data), headers=self.__default_headers, verify = self.__verify) return PackitClient.handle_response(response) def __authenticate(self): diff --git a/test/unit/test_packit_client.py b/test/unit/test_packit_client.py index 710731c..1dfcbf5 100644 --- a/test/unit/test_packit_client.py +++ b/test/unit/test_packit_client.py @@ -22,6 +22,13 @@ def mock_auth(mock_montagu_api_class, requests_mock): mock_montagu_api.token = "test-montagu-token" requests_mock.get(f"{PACKIT_URL}/api/auth/login/montagu", text = json.dumps({"token": "test-packit-token"} )) +def assert_expected_packit_api_request(requests_mock, index, method, url, text = None): + req = requests_mock.request_history[index] + assert req.headers["Authorization"] == "Bearer test-packit-token" + assert req.method == method + assert req.url == url + assert req.text == text + @patch("montagu.MontaguAPI") def test_authenticates_on_init(MockMontaguAPI, requests_mock): mock_auth(MockMontaguAPI, requests_mock) @@ -37,15 +44,40 @@ def test_authenticates_on_init(MockMontaguAPI, requests_mock): assert sut.token == "test-packit-token" -#@patch("montagu.MontaguAPI") -#def test_run(): -# mock_auth(MockMontaguAPI, requests_mock) +@patch("montagu.MontaguAPI") +def test_run(MockMontaguAPI, requests_mock): + mock_auth(MockMontaguAPI, requests_mock) + mock_branches_response = { "branches": [ + { "name": "some_other_branch", "commitHash": "xyz987" }, + { "name": "main", "commitHash": "abc123" } + ] } + requests_mock.get(f"{PACKIT_URL}/api/runner/git/branches", text = json.dumps(mock_branches_response)) + requests_mock.post(f"{PACKIT_URL}/api/runner/run", text = json.dumps({ "taskId": "test-task-id" })) -#def test_reauthenticates_on_401 + sut = PackitClient(config) + task_id = sut.run("test-packet-group", {"a": 1, "b": 2}) -#def test_raises_exception_on_unexpected_status + assert task_id == "test-task-id" + assert_expected_packit_api_request(requests_mock, 1, "GET", f"{PACKIT_URL}/api/runner/git/branches") + expected_run_payload = json.dumps({ + "name": "test-packet-group", + "parameters": {"a": 1, "b": 2}, + "branch": "main", + "hash": "abc123" + }) + assert_expected_packit_api_request(requests_mock, 2, "POST", f"{PACKIT_URL}/api/runner/run", expected_run_payload) + + +@patch("montagu.MontaguAPI") +def test_poll_status(MockMontaguAPI, requests_mock): + mock_auth(MockMontaguAPI, requests_mock) + mock_poll_response = { "status": "RUNNING" } + requests_mock.get(f"{PACKIT_URL}/api/runner/status/test-task-id", text = json.dumps(mock_poll_response)) + + sut = PackitClient(config) + resp = sut.poll("test-task-id") -#def test_poll_status + assert resp == mock_poll_response @patch("montagu.MontaguAPI") def test_kill_task(MockMontaguAPI, requests_mock): @@ -57,8 +89,37 @@ def test_kill_task(MockMontaguAPI, requests_mock): resp = sut.kill_task("test-task-id") assert resp == mock_kill_response - req = requests_mock.request_history[1] - assert req.headers["Authorization"] == "Bearer test-packit-token" + assert_expected_packit_api_request(requests_mock, 1, "POST", f"{PACKIT_URL}/api/runner/cancel/test-task-id") + + +@patch("montagu.MontaguAPI") +def test_publish(MockMontaguAPI, requests_mock): + mock_auth(MockMontaguAPI, requests_mock) + # publish polls for the packit - return 404 on first poll, 200 on second poll + requests_mock.get(f"{PACKIT_URL}/api/packets/test-packet-id", [ + { "text": json.dumps({ "error": { "detail": "not found" } }), "status_code": 404}, + { "text": json.dumps({ "id": "test-packet-id", "name": "A packet" }), "status_code": 200 } + ]) + expected_permissions_payload = json.dumps({ + "addPermissions": [{ "permission": "packet.read", "packetId": "test-packet-id" }], + "removePermissions": [] + }) + requests_mock.put(f"{PACKIT_URL}/api/roles/test-role-1/permissions", json = "null") + requests_mock.put(f"{PACKIT_URL}/api/roles/test-role-2/permissions", text = "null") + + sut = PackitClient(config) + result = sut.publish("A packet", "test-packet-id", ["test-role-1", "test-role-2"]) + assert_expected_packit_api_request(requests_mock, 1, "GET", f"{PACKIT_URL}/api/packets/test-packet-id", None) + assert_expected_packit_api_request(requests_mock, 2, "GET", f"{PACKIT_URL}/api/packets/test-packet-id", None) + + assert_expected_packit_api_request(requests_mock, 3, "PUT", f"{PACKIT_URL}/api/roles/test-role-1/permissions", expected_permissions_payload) + assert_expected_packit_api_request(requests_mock, 4, "PUT", f"{PACKIT_URL}/api/roles/test-role-2/permissions", expected_permissions_payload) + assert result + +#def test_raises_exception_when_auth_fails + +#def test_reauthenticates_on_401 + +#def test_raises_exception_on_unexpected_status -#def test_publish From d3cd7580b7eaaf4312a2ce93e8ca8faaf047d08d Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 15 Jul 2025 11:25:02 +0100 Subject: [PATCH 14/36] more unit tests --- src/packit_client.py | 2 +- test/unit/test_packit_client.py | 54 +++++++++++++++++++++++++++++---- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/packit_client.py b/src/packit_client.py index 4fce56d..10fd61c 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -130,7 +130,7 @@ def do_publish_to_role(role): } self.__put(f"/roles/{role}/permissions", data) - self.__wait_for_packet_to_be_imported(packet_id) + self.__execute(lambda: self.__wait_for_packet_to_be_imported(packet_id)) logging.info(f"Publishing packet {name}({packet_id})") success = True diff --git a/test/unit/test_packit_client.py b/test/unit/test_packit_client.py index 1dfcbf5..e66ee1b 100644 --- a/test/unit/test_packit_client.py +++ b/test/unit/test_packit_client.py @@ -1,7 +1,9 @@ import json import montagu +import pytest import requests_mock from src.packit_client import PackitClient +from src.packit_client_exception import PackitClientException from unittest.mock import patch, MagicMock PACKIT_URL = "http://test-packit" @@ -29,6 +31,13 @@ def assert_expected_packit_api_request(requests_mock, index, method, url, text = assert req.url == url assert req.text == text +def assert_expected_packit_auth_request(requests_mock, index): + auth_req = requests_mock.request_history[index] + assert auth_req.method == "GET" + assert auth_req.url == f"{PACKIT_URL}/api/auth/login/montagu" + assert auth_req.headers["Authorization"] == "Bearer test-montagu-token" + + @patch("montagu.MontaguAPI") def test_authenticates_on_init(MockMontaguAPI, requests_mock): mock_auth(MockMontaguAPI, requests_mock) @@ -37,9 +46,7 @@ def test_authenticates_on_init(MockMontaguAPI, requests_mock): MockMontaguAPI.assert_called_with("http://test-montagu", "test.montagu.user", "montagu_password") auth_call = requests_mock.request_history[0] - assert auth_call.method == "GET" - assert auth_call.url == f"{PACKIT_URL}/api/auth/login/montagu" - assert auth_call.headers["Authorization"] == "Bearer test-montagu-token" + assert_expected_packit_auth_request(requests_mock, 0) assert sut.auth_success assert sut.token == "test-packit-token" @@ -116,10 +123,45 @@ def test_publish(MockMontaguAPI, requests_mock): assert_expected_packit_api_request(requests_mock, 4, "PUT", f"{PACKIT_URL}/api/roles/test-role-2/permissions", expected_permissions_payload) assert result -#def test_raises_exception_when_auth_fails +@patch("montagu.MontaguAPI") +def test_sets_auth_success_to_false_when_auth_fails(MockMontaguAPI, requests_mock): + requests_mock.get(f"{PACKIT_URL}/api/auth/login/montagu", status_code = 401, text = json.dumps({"error": "Unauthorized"})) + sut = PackitClient(config) + assert not sut.auth_success + +@patch("montagu.MontaguAPI") +def test_reauthenticates_on_401(MockMontaguAPI, requests_mock): + # Reauthentication should take place as part of the __execute wrapper used + # with all methods which require authentication - here we just test a sample + # method to check the pattern works. + mock_auth(MockMontaguAPI, requests_mock) + mock_successful_kill_response = { "status": "dead" } + requests_mock.post(f"{PACKIT_URL}/api/runner/cancel/test-task-id", [ + {"status_code": 401, "text": json.dumps({"error": "Unauthorized"})}, + {"status_code": 200, "text": json.dumps(mock_successful_kill_response)} + ]) + + sut = PackitClient(config) + resp = sut.kill_task("test-task-id") + + assert resp == mock_successful_kill_response + + assert_expected_packit_api_request(requests_mock, 1, "POST", f"{PACKIT_URL}/api/runner/cancel/test-task-id") + assert_expected_packit_auth_request(requests_mock, 2) + assert_expected_packit_api_request(requests_mock, 3, "POST", f"{PACKIT_URL}/api/runner/cancel/test-task-id") -#def test_reauthenticates_on_401 +@patch("montagu.MontaguAPI") +def test_raises_exception_on_unexpected_status(MockMontaguAPI, requests_mock): + mock_auth(MockMontaguAPI, requests_mock) + # execute does not tolerate status codes other than 401 - should get an exception + bad_response = {"error": "Bad request"} + requests_mock.get(f"{PACKIT_URL}/api/runner/status/test-task-id", status_code = 400, text = json.dumps(bad_response)) + + sut = PackitClient(config) + with pytest.raises(PackitClientException) as exc_info: + sut.poll("test-task-id") + assert exc_info.value.response.status_code == 400 + assert exc_info.value.response.json() == bad_response -#def test_raises_exception_on_unexpected_status From bfddf84b5b9ccec7ab31a4d51de0f3e716f82c8d Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 15 Jul 2025 16:16:16 +0100 Subject: [PATCH 15/36] fix first run report unit test --- src/utils/run_reports.py | 5 +- test/integration/test_run_reports.py | 6 +- test/unit/test_create_ticket.py | 16 +- test/unit/test_orderlyweb_client_wrapper.py | 121 ------------ test/unit/test_packit_client.py | 12 +- test/unit/test_run_reports.py | 201 ++++++++++++-------- 6 files changed, 135 insertions(+), 226 deletions(-) delete mode 100644 test/unit/test_orderlyweb_client_wrapper.py diff --git a/src/utils/run_reports.py b/src/utils/run_reports.py index b3e6ada..7a80b07 100644 --- a/src/utils/run_reports.py +++ b/src/utils/run_reports.py @@ -42,7 +42,7 @@ def run_reports(packit, group, disease, touchstone, config, reports, already_running = running_reports_repo.get(group, disease, report.name) if already_running is not None: try: - logging.info("Killing already running task: {}. Key is {}" + logging.info("Killing already running task: {}. Key is {}." .format(report.name, already_running)) packit.kill_task(already_running) except Exception as ex: @@ -62,7 +62,7 @@ def run_reports(packit, group, disease, touchstone, config, reports, running_reports[key] = report # Save key to shared data - may be killed by subsequent task running_reports_repo.set(group, disease, report.name, key) - logging.info("Running report: {} with parameters {}. Key is {}. " + logging.info("Running report: {} with parameters {}. Key is {}." .format(report.name, params_to_string(parameters), key)) except Exception as ex: @@ -89,6 +89,7 @@ def run_reports(packit, group, disease, touchstone, config, reports, report_config = next(filter(lambda report: report.name == name, reports), None) if report_config is not None: + logging.info("Publishing report packet {} ({})".format(name, packet_id)) published = publish_report(packit, name, packet_id, report_config.publish_roles) if published: logging.info( diff --git a/test/integration/test_run_reports.py b/test/integration/test_run_reports.py index 024dda2..773f613 100644 --- a/test/integration/test_run_reports.py +++ b/test/integration/test_run_reports.py @@ -7,9 +7,9 @@ def test_run_reports_handles_error(): reports = [ ReportConfig("nonexistent", None, ["test1@test.com"], "subject1", - 600, "a.ssignee"), - ReportConfig("diagnostic", {}, ["test2@test.com"], "subject2", 600, - "a.ssignee")] + "a.ssignee", ["Funders"]), + ReportConfig("diagnostic", {}, ["test2@test.com"], "subject2", + "a.ssignee", ["Funders"])] config = Config() wrapper = OrderlyWebClientWrapper(config) success = {} diff --git a/test/unit/test_create_ticket.py b/test/unit/test_create_ticket.py index 7485bba..d03a66d 100644 --- a/test/unit/test_create_ticket.py +++ b/test/unit/test_create_ticket.py @@ -14,7 +14,7 @@ def test_tags_created(): report = ReportConfig("TEST", {}, ["to@example.com"], - "Hi", 100, "a.ssignee") + "Hi", "a.ssignee", ["Funders"]) mock_config: Config = MockConfig() mock_client = Mock(spec=YTClient("", "")) mock_client.create_issue = Mock(return_value="ISSUE") @@ -28,7 +28,7 @@ def test_tags_created(): def test_tags_not_created_if_exists(): report = ReportConfig("TEST", {}, ["to@example.com"], - "Hi", 100, "a.ssignee") + "Hi", "a.ssignee", ["Funders"]) mock_config: Config = MockConfig() mock_client = Mock(spec=YTClient("", "")) mock_client.create_issue = Mock(return_value="ISSUE") @@ -41,7 +41,7 @@ def test_tags_not_created_if_exists(): def test_create_ticket_with_version(): report = ReportConfig("TEST", {}, ["to@example.com"], - "Hi", 100, "a.ssignee") + "Hi", "a.ssignee", ["Funders"]) mock_config: Config = MockConfig() mock_client = Mock(spec=YTClient("", "")) mock_client.create_issue = Mock(return_value="ISSUE") @@ -52,7 +52,7 @@ def test_create_ticket_with_version(): expected_create = call(Project(id="78-0"), "Check & share diag report with g1 (d1) t1", "Report run triggered by upload to scenario: s1. " - "http://orderly-web/report/TEST/1234/") + "http://test-packit/TEST/1234/") mock_client.create_issue.assert_has_calls([expected_create]) expected_command_query = \ "for a.ssignee implementer a.ssignee tag g1 tag d1 tag t1 tag TEST" @@ -63,7 +63,7 @@ def test_create_ticket_with_version(): def test_create_ticket_without_version(): report = ReportConfig("TEST", {}, ["to@example.com"], - "Hi", 100, "a.ssignee") + "Hi", "a.ssignee", ["Funders"]) mock_config: Config = MockConfig() mock_client = Mock(spec=YTClient("", "")) mock_client.create_issue = Mock(return_value="ISSUE") @@ -87,7 +87,7 @@ def test_create_ticket_without_version(): @patch("src.task_run_diagnostic_reports.logging") def test_create_ticket_logs_errors(logging): report = ReportConfig("TEST", {}, ["to@example.com"], - "Hi", 100, "a.ssignee") + "Hi", "a.ssignee", ["Funders"]) mock_config: Config = MockConfig() mock_client = Mock(spec=YTClient("", "")) mock_client.get_issues = Mock(return_value=[]) @@ -101,7 +101,7 @@ def test_create_ticket_logs_errors(logging): def test_update_ticket(): report = ReportConfig("TEST", {}, ["to@example.com"], - "Hi", 100, "a.ssignee") + "Hi", "a.ssignee", ["Funders"]) mock_config: Config = MockConfig() mock_client = Mock(spec=YTClient("", "")) mock_client.get_issues = Mock(return_value=["ISSUE"]) @@ -111,5 +111,5 @@ def test_update_ticket(): expected_command = call("ISSUE", "Check & share diag report with g1 (d1) t1", "Report run triggered by upload to scenario: s1. " - "http://orderly-web/report/TEST/1234/") + "http://test-packit/TEST/1234/") mock_client.update_issue.assert_has_calls([expected_command]) diff --git a/test/unit/test_orderlyweb_client_wrapper.py b/test/unit/test_orderlyweb_client_wrapper.py deleted file mode 100644 index f20f2ac..0000000 --- a/test/unit/test_orderlyweb_client_wrapper.py +++ /dev/null @@ -1,121 +0,0 @@ -from src.utils.run_reports import run_reports -from src.config import ReportConfig -from orderlyweb_api import ReportStatusResult -from unittest.mock import patch, call -from src.orderlyweb_client_wrapper import OrderlyWebClientWrapper -from test import ExceptionMatching -from test.unit.test_run_reports import MockConfig, MockRunningReportRepository - -reports = [ReportConfig("r1", None, ["r1@example.com"], - "Subj: r1", 1000, "a.ssignee")] - -report_response = ReportStatusResult({"status": "success", - "version": "r1-version", - "output": None}) - -group = "test_group" -disease = "test_disease" -touchstone = "2021test-1" - - -class MockOrderlyWebAPIWithExpiredToken: - - def run_report(self, name, params, timeout): - raise Exception("Token expired") - - def report_status(self, key): - raise Exception("Token expired") - - def publish_report(self, name, version): - raise Exception("Token expired") - - -class MockOrderlyWebAPIWithValidToken: - - def run_report(self, name, params, timeout): - return "r1-key" - - def report_status(self, key): - return report_response - - def publish_report(self, name, version): - return True - - -class MockReturnAuthorisedClient: - def __init__(self): - self.callCount = 0 - - def auth(self, config): - if self.callCount == 0: - self.callCount = 1 - # if this is the first call, return an expired token error - return MockOrderlyWebAPIWithExpiredToken() - else: - # on the second call, return success reponses - return MockOrderlyWebAPIWithValidToken() - - -@patch("src.utils.run_reports.logging") -def test_retries_when_token_expired(logging): - auth = MockReturnAuthorisedClient().auth - wrapper = OrderlyWebClientWrapper(None, auth) - success = {} - error = {} - - def success_callback(report, version): - success["called"] = True - - def error_callback(report, version=None): - error["called"] = True - - mock_running_reports = MockRunningReportRepository() - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), - reports, success_callback, error_callback, - mock_running_reports) - - assert versions == {"r1-version": {"published": True, "report": "r1"}} - logging.info.assert_has_calls([ - call("Running report: r1 with parameters touchstone=2021test-1," - " touchstone_name=2021test. " - "Key is r1-key. Timeout is 1000s."), - call("Success for key r1-key. New version is r1-version"), - call("Publishing report version r1-r1-version"), - call("Successfully published report version r1-r1-version") - ], any_order=False) - - assert success["called"] is True - assert len(error) == 0 - - -@patch("src.utils.run_reports.logging") -@patch("src.orderlyweb_client_wrapper.logging") -def test_handles_auth_errors(logging_ow, logging_reports): - wrapper = OrderlyWebClientWrapper({}) - success = {} - error = {} - - def success_callback(report, version): - success["called"] = True - - def error_callback(report, version=None): - error["called"] = True - - mock_running_reports = MockRunningReportRepository() - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), - reports, success_callback, error_callback, - mock_running_reports) - - # the wrapper will have an auth failure because no auth config - # supplied - expected_error = AttributeError( - "'dict' object has no attribute 'montagu_url'") - logging_ow.exception.assert_called_once_with( - ExceptionMatching(expected_error)) - - logging_reports.error.assert_called_once_with( - "Orderlyweb authentication failed; could not begin task") - - assert len(success) == 0 - assert len(versions) == 0 - assert error["called"] is True diff --git a/test/unit/test_packit_client.py b/test/unit/test_packit_client.py index e66ee1b..959abff 100644 --- a/test/unit/test_packit_client.py +++ b/test/unit/test_packit_client.py @@ -4,19 +4,9 @@ import requests_mock from src.packit_client import PackitClient from src.packit_client_exception import PackitClientException +from test.unit.test_run_reports import MockConfig, PACKIT_URL from unittest.mock import patch, MagicMock -PACKIT_URL = "http://test-packit" - -class MockConfig: - def __init__(self): - self.disable_certificate_verify = False - self.montagu_url = "http://test-montagu" - self.montagu_user = "test.montagu.user" - self.montagu_password = "montagu_password" - self.packit_url = PACKIT_URL - - config = MockConfig() def mock_auth(mock_montagu_api_class, requests_mock): diff --git a/test/unit/test_run_reports.py b/test/unit/test_run_reports.py index 747563b..1fc6085 100644 --- a/test/unit/test_run_reports.py +++ b/test/unit/test_run_reports.py @@ -5,9 +5,9 @@ from src.orderlyweb_client_wrapper import OrderlyWebClientWrapper reports = [ReportConfig("r1", None, ["r1@example.com"], "Subj: r1", - 1000, "a.ssignee"), + "a.ssignee", ["Funders"]), ReportConfig("r2", {"p1": "v1"}, ["r2@example.com"], "Subj: r2", - 2000, "a.ssignee")] + "a.ssignee", ["Tech"])] expected_params = { "r1": {"touchstone": "2021test-1", "touchstone_name": "2021test"}, @@ -15,42 +15,34 @@ "touchstone_name": "2021test"} } -expected_timeouts = { - "r1": 1000, - "r2": 2000 -} - group = "test_group" disease = "test_disease" touchstone = "2021test-1" expected_run_rpt_1_log = "Running report: r1 with parameters " \ "touchstone=2021test-1, touchstone_name=2021test. " \ - "Key is r1-key. Timeout is 1000s." + "Key is r1-key." expected_run_rpt_2_log = "Running report: r2 with parameters p1=v1, " \ "touchstone=2021test-1, touchstone_name=2021test. " \ - "Key is r2-key. Timeout is 2000s." + "Key is r2-key." @patch("src.utils.run_reports.logging") def test_run_reports(logging): run_successfully = ["r1", "r2"] report_responses = { - "r1-key": [ReportStatusResult({"status": "success", - "version": "r1-version", - "output": None})], - "r2-key": [ReportStatusResult({"status": "success", - "version": "r2-version", - "output": None})] + "r1-key": [{"status": "COMPLETE", + "packetId": "r1-version"}], + "r2-key": [{"status": "COMPLETE", + "packetId": "r2-version"}] } - ow = MockOrderlyWebAPI(run_successfully, report_responses, - expected_params, expected_timeouts) - wrapper = OrderlyWebClientWrapper(None, lambda x: ow) + packit = MockPackitClient(expected_params, run_successfully, report_responses) success = {} error = {"called": False} + # TODO: replace with mock and test parameters def success_callback(report, version): success["called"] = True @@ -59,7 +51,7 @@ def error_callback(report, message): mock_running_reports = MockRunningReportRepository() - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), + versions = run_reports(packit, group, disease, touchstone, MockConfig(), reports, success_callback, error_callback, mock_running_reports) @@ -71,16 +63,16 @@ def error_callback(report, message): logging.info.assert_has_calls([ call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), - call("Success for key r1-key. New version is r1-version"), - call("Publishing report version r1-r1-version"), - call("Successfully published report version r1-r1-version"), - call("Success for key r2-key. New version is r2-version"), - call("Publishing report version r2-r2-version"), - call("Successfully published report version r2-r2-version") + call("Success for key r1-key. New packet id is r1-version"), + call("Publishing report packet r1 (r1-version)"), + call("Successfully published report packet r1 (r1-version)"), + call("Success for key r2-key. New packet id is r2-version"), + call("Publishing report packet r2 (r2-version)"), + call("Successfully published report packet r2 (r2-version)") ], any_order=False) mock_running_reports.assert_expected_calls() - ow.kill_report.assert_not_called() + packit.kill_task.assert_not_called() assert success["called"] is True assert error["called"] is False @@ -173,12 +165,12 @@ def error_callback(report, message): call(expected_run_rpt_1_log), call("Killing already running report: r2. Key is r2-old-key"), call(expected_run_rpt_2_log), - call("Success for key r1-key. New version is r1-version"), - call("Publishing report version r1-r1-version"), - call("Successfully published report version r1-r1-version"), - call("Success for key r2-key. New version is r2-version"), - call("Publishing report version r2-r2-version"), - call("Successfully published report version r2-r2-version") + call("Success for key r1-key. New packet id is r1-version"), + call("Publishing report packet r1 (r1-version)"), + call("Successfully published report packet r1 (r1-version)"), + call("Success for key r2-key. New packet id is r2-version"), + call("Publishing report packet r2 (r2-version)"), + call("Successfully published report packet r2 (r2-version)") ], any_order=False) mock_running_reports.assert_expected_calls() @@ -230,12 +222,12 @@ def error_callback(report, message): logging.info.assert_has_calls([ call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), - call("Success for key r1-key. New version is r1-version"), - call("Publishing report version r1-r1-version"), - call("Successfully published report version r1-r1-version"), - call("Success for key r2-key. New version is r2-version"), - call("Publishing report version r2-r2-version"), - call("Successfully published report version r2-r2-version") + call("Success for key r1-key. New packet id is r1-version"), + call("Publishing report version r1 (r1-version)"), + call("Successfully published report packet r1 (r1-version)"), + call("Success for key r2-key. New packet id is r2-version"), + call("Publishing report version r2 (r2-version)"), + call("Successfully published report packet r2 (r2-version)") ], any_order=False) mock_running_reports.assert_expected_calls() @@ -289,12 +281,12 @@ def error_callback(report, message): logging.info.assert_has_calls([ call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), - call("Success for key r2-key. New version is r2-version"), - call("Publishing report version r2-r2-version"), - call("Successfully published report version r2-r2-version"), - call("Success for key r1-key. New version is r1-version"), - call("Publishing report version r1-r1-version"), - call("Successfully published report version r1-r1-version") + call("Success for key r2-key. New packet id is r2-version"), + call("Publishing report version r2 (r2-version)"), + call("Successfully published report packet r2 (r2-version)"), + call("Success for key r1-key. New packet id is r1-version"), + call("Publishing report version r1 (r1-version)"), + call("Successfully published report packet r1 (r1-version)") ], any_order=False) mock_running_reports.assert_expected_calls() @@ -312,8 +304,8 @@ def test_run_reports_with_run_error(logging): "version": "r2-version", "output": None})] } - ow = MockOrderlyWebAPI(run_successfully, report_responses, - expected_params, expected_timeouts) + packit = MockOrderlyWebAPI(run_successfully, report_responses, + expected_params) wrapper = OrderlyWebClientWrapper(None, lambda x: ow) success = {} error = {"called": False} @@ -337,9 +329,9 @@ def error_callback(report, message): expected_err = "test-run-error: r1" logging.info.assert_has_calls([ call(expected_run_rpt_2_log), - call("Success for key r2-key. New version is r2-version"), - call("Publishing report version r2-r2-version"), - call("Successfully published report version r2-r2-version") + call("Success for key r2-key. New packet id is r2-version"), + call("Publishing report version r2 (r2-version)"), + call("Successfully published report packet r2 (r2-version)") ], any_order=False) args, kwargs = logging.exception.call_args assert str(args[0]) == expected_err @@ -398,9 +390,9 @@ def error_callback(report, message): logging.info.assert_has_calls([ call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), - call("Success for key r2-key. New version is r2-version"), - call("Publishing report version r2-r2-version"), - call("Successfully published report version r2-r2-version") + call("Success for key r2-key. New packet id is r2-version"), + call("Publishing report version r2 (r2-version)"), + call("Successfully published report packet r2 (r2-version)") ], any_order=False) args, kwargs = logging.exception.call_args assert str(args[0]) == "test-status-error: r1-key" @@ -449,9 +441,9 @@ def error_callback(report, message): logging.info.assert_has_calls([ call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), - call("Success for key r1-key. New version is r1-version"), - call("Publishing report version r1-r1-version"), - call("Successfully published report version r1-r1-version") + call("Success for key r1-key. New packet id is r1-version"), + call("Publishing report version r1 (r1-version)"), + call("Successfully published report packet r1 (r1-version)") ], any_order=False) logging.error.assert_has_calls([ call("Failure for key r2-key. Status: error") @@ -500,9 +492,9 @@ def error_callback(report, message): logging.info.assert_has_calls([ call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), - call("Success for key r1-key. New version is r1-version"), - call("Publishing report version r1-r1-version"), - call("Successfully published report version r1-r1-version") + call("Success for key r1-key. New packet id is r1-version"), + call("Publishing report version r1 (r1-version)"), + call("Successfully published report packet r1 (r1-version)") ], any_order=False) logging.error.assert_has_calls([ call("Failure for key r2-key. Status: interrupted") @@ -550,15 +542,15 @@ def error_callback(report, message): "r2-version": {"published": False, "report": "r2"} } - expected_err = "Failed to publish report version r2-r2-version" + expected_err = "Failed to publish report version r2 (r2-version)" logging.info.assert_has_calls([ call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), - call("Success for key r1-key. New version is r1-version"), - call("Publishing report version r1-r1-version"), - call("Successfully published report version r1-r1-version"), - call("Success for key r2-key. New version is r2-version"), - call("Publishing report version r2-r2-version") + call("Success for key r1-key. New packet id is r1-version"), + call("Publishing report version r1 (r1-version)"), + call("Successfully published report packet r1 (r1-version)"), + call("Success for key r2-key. New packet id is r2-version"), + call("Publishing report version r2 (r2-version)") ], any_order=False) logging.error.assert_has_calls([ call(expected_err) @@ -592,41 +584,72 @@ def assert_expected_calls(self): call(group, disease, "r2", "r2-key") ], any_order=False) - -class MockOrderlyWebAPI: - def __init__(self, run_successfully, report_responses, expected_params, - expected_timeouts, fail_publish=None): - if fail_publish is None: - fail_publish = [] +class MockPackitClient: + def __init__(self, expected_params, run_successfully, report_responses, fail_publish = []): + self.auth_success = True + self.expected_params = expected_params self.run_successfully = run_successfully self.report_responses = report_responses - self.expected_params = expected_params - self.expected_timeouts = expected_timeouts self.fail_publish = fail_publish - self.kill_report = Mock() + self.kill_task = Mock() - def run_report(self, name, params, timeout): + def run(self, name, params): assert params == self.expected_params[name] - assert timeout == self.expected_timeouts[name] if name in self.run_successfully: return name + "-key" else: raise Exception("test-run-error: " + name) - def report_status(self, key): + def poll(self, key): if key in self.report_responses and \ len(self.report_responses[key]) > 0: return self.report_responses[key].pop(0) else: raise Exception("test-status-error: " + key) - def publish_report(self, name, version): + def publish(self, name, packit_id, roles): if name in self.fail_publish: raise Exception("Publish failed") else: return True +#class MockOrderlyWebAPI: +# def __init__(self, run_successfully, report_responses, expected_params, +# expected_timeouts, fail_publish=None): +# if fail_publish is None: +# fail_publish = [] +# self.run_successfully = run_successfully +# self.report_responses = report_responses +# self.expected_params = expected_params +# self.expected_timeouts = expected_timeouts +# self.fail_publish = fail_publish +# self.kill_report = Mock() + +# def run_report(self, name, params, timeout): +# assert params == self.expected_params[name] +# assert timeout == self.expected_timeouts[name] +# if name in self.run_successfully: +# return name + "-key" +# else: +# raise Exception("test-run-error: " + name)# + +# def report_status(self, key): +# if key in self.report_responses and \ +# len(self.report_responses[key]) > 0: +# return self.report_responses[key].pop(0) +# else: +# raise Exception("test-status-error: " + key) + +# def publish_report(self, name, version): +# if name in self.fail_publish: +# raise Exception("Publish failed") +# else: +# return True + + +PACKIT_URL = "http://test-packit" + class MockConfig: def __init__(self, use_additional_recipients=True): @@ -656,10 +679,26 @@ def smtp_user(self): def smtp_password(self): return None - @property - def orderlyweb_url(self): - return "http://orderly-web" - @property def youtrack_token(self): return "12345" + + @property + def packit_url(self): + return PACKIT_URL + + @property + def disable_certificate_verify(self): + return False + + @property + def montagu_url(self): + return "http://test-montagu" + + @property + def montagu_user(self): + return "test.montagu.user" + + @property + def montagu_password(self): + return "montagu_password" From 6608905a236b1dbc0e904f9cad0c8c910c6bb682 Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 15 Jul 2025 17:34:48 +0100 Subject: [PATCH 16/36] fix all run_reports unit tests --- test/unit/test_run_reports.py | 391 ++++++++++------------------------ 1 file changed, 112 insertions(+), 279 deletions(-) diff --git a/test/unit/test_run_reports.py b/test/unit/test_run_reports.py index 1fc6085..9bf300a 100644 --- a/test/unit/test_run_reports.py +++ b/test/unit/test_run_reports.py @@ -39,15 +39,8 @@ def test_run_reports(logging): } packit = MockPackitClient(expected_params, run_successfully, report_responses) - success = {} - error = {"called": False} - - # TODO: replace with mock and test parameters - def success_callback(report, version): - success["called"] = True - - def error_callback(report, message): - error["called"] = message + success_callback = Mock() + error_callback = Mock() mock_running_reports = MockRunningReportRepository() @@ -74,19 +67,20 @@ def error_callback(report, message): mock_running_reports.assert_expected_calls() packit.kill_task.assert_not_called() - assert success["called"] is True - assert error["called"] is False + success_callback.assert_has_calls([ + call(reports[0], "r1-version"), + call(reports[1], "r2-version") + ]) + error_callback.assert_not_called() def test_run_reports_with_multi_hyphen_touchstone(): run_successfully = ["r1", "r2"] report_responses = { - "r1-key": [ReportStatusResult({"status": "success", - "version": "r1-version", - "output": None})], - "r2-key": [ReportStatusResult({"status": "success", - "version": "r2-version", - "output": None})] + "r1-key": [{"status": "COMPLETE", + "packetId": "r1-version"}], + "r2-key": [{"status": "COMPLETE", + "packetId": "r2-version"}] } multi_touchstone = "2021test-extra-1" @@ -96,22 +90,13 @@ def test_run_reports_with_multi_hyphen_touchstone(): "r2": {"p1": "v1", "touchstone": "2021test-extra-1", "touchstone_name": "2021test-extra"} } - ow = MockOrderlyWebAPI(run_successfully, report_responses, - expected_multi_params, expected_timeouts) - wrapper = OrderlyWebClientWrapper(None, lambda x: ow) - - success = {} - error = {"called": False} - - def success_callback(report, version): - success["called"] = True - - def error_callback(report, message): - error["called"] = message + packit = MockPackitClient(expected_multi_params, run_successfully, report_responses) + success_callback = Mock() + error_callback = Mock() mock_running_reports = MockRunningReportRepository() - versions = run_reports(wrapper, group, disease, multi_touchstone, + versions = run_reports(packit, group, disease, multi_touchstone, MockConfig(), reports, success_callback, error_callback, mock_running_reports) @@ -120,38 +105,31 @@ def error_callback(report, message): "r1-version": {"published": True, "report": "r1"}, "r2-version": {"published": True, "report": "r2"} } - assert success["called"] is True - assert error["called"] is False + success_callback.assert_has_calls([ + call(reports[0], "r1-version"), + call(reports[1], "r2-version") + ]) + error_callback.assert_not_called() @patch("src.utils.run_reports.logging") def test_run_reports_kills_currently_running(logging): run_successfully = ["r1", "r2"] report_responses = { - "r1-key": [ReportStatusResult({"status": "success", - "version": "r1-version", - "output": None})], - "r2-key": [ReportStatusResult({"status": "success", - "version": "r2-version", - "output": None})] + "r1-key": [{"status": "COMPLETE", + "packetId": "r1-version"}], + "r2-key": [{"status": "COMPLETE", + "packetId": "r2-version"}] } - ow = MockOrderlyWebAPI(run_successfully, report_responses, - expected_params, expected_timeouts) - wrapper = OrderlyWebClientWrapper(None, lambda x: ow) - success = {} - error = {"called": False} - - def success_callback(report, version): - success["called"] = True - - def error_callback(report, message): - error["called"] = message + packit = MockPackitClient(expected_params, run_successfully, report_responses) + success_callback = Mock() + error_callback = Mock() mock_running_reports = \ MockRunningReportRepository(["r1-old-key", "r2-old-key"]) - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), + versions = run_reports(packit, group, disease, touchstone, MockConfig(), reports, success_callback, error_callback, mock_running_reports) @@ -161,9 +139,9 @@ def error_callback(report, message): } logging.info.assert_has_calls([ - call("Killing already running report: r1. Key is r1-old-key"), + call("Killing already running task: r1. Key is r1-old-key."), call(expected_run_rpt_1_log), - call("Killing already running report: r2. Key is r2-old-key"), + call("Killing already running task: r2. Key is r2-old-key."), call(expected_run_rpt_2_log), call("Success for key r1-key. New packet id is r1-version"), call("Publishing report packet r1 (r1-version)"), @@ -175,101 +153,35 @@ def error_callback(report, message): mock_running_reports.assert_expected_calls() - ow.kill_report.assert_has_calls([ + packit.kill_task.assert_has_calls([ call("r1-old-key"), call("r2-old-key") ], any_order=False) - assert success["called"] is True - assert error["called"] is False - - -@patch("src.utils.run_reports.logging") -def test_run_reports_with_additional_recipients(logging): - run_successfully = ["r1", "r2"] - report_responses = { - "r1-key": [ReportStatusResult({"status": "success", - "version": "r1-version", - "output": None})], - "r2-key": [ReportStatusResult({"status": "success", - "version": "r2-version", - "output": None})] - } - - ow = MockOrderlyWebAPI(run_successfully, report_responses, - expected_params, expected_timeouts) - wrapper = OrderlyWebClientWrapper(None, lambda x: ow) - success = {} - error = {"called": False} - - def success_callback(report, version): - success["called"] = True - - def error_callback(report, message): - error["called"] = message - - mock_running_reports = MockRunningReportRepository() - - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), - reports, success_callback, error_callback, - mock_running_reports) - - assert versions == { - "r1-version": {"published": True, "report": "r1"}, - "r2-version": {"published": True, "report": "r2"} - } - - logging.info.assert_has_calls([ - call(expected_run_rpt_1_log), - call(expected_run_rpt_2_log), - call("Success for key r1-key. New packet id is r1-version"), - call("Publishing report version r1 (r1-version)"), - call("Successfully published report packet r1 (r1-version)"), - call("Success for key r2-key. New packet id is r2-version"), - call("Publishing report version r2 (r2-version)"), - call("Successfully published report packet r2 (r2-version)") - ], any_order=False) - - mock_running_reports.assert_expected_calls() - ow.kill_report.assert_not_called() - - assert success["called"] is True - assert error["called"] is False + success_callback.assert_has_calls([ + call(reports[0], "r1-version"), + call(reports[1], "r2-version") + ]) + error_callback.assert_not_called() @patch("src.utils.run_reports.logging") def test_run_reports_finish_on_different_poll_cycles(logging): run_successfully = ["r1", "r2"] report_responses = { - "r1-key": [ReportStatusResult({"status": "running", - "version": None, - "output": None}), - ReportStatusResult({"status": "running", - "version": None, - "output": None}), - ReportStatusResult({"status": "success", - "version": "r1-version", - "output": None}) + "r1-key": [{"status": "RUNNING", "packetId": None}, + {"status": "RUNNING", "packetId": None}, + {"status": "COMPLETE", "packetId": "r1-version"} ], - "r2-key": [ReportStatusResult({"status": "success", - "version": "r2-version", - "output": None})] + "r2-key": [{"status": "COMPLETE", "packetId": "r2-version"}] } - ow = MockOrderlyWebAPI(run_successfully, report_responses, - expected_params, expected_timeouts) - wrapper = OrderlyWebClientWrapper(None, lambda x: ow) - success = {} - error = {"called": False} - - def success_callback(report, version): - success["called"] = True - - def error_callback(report, message): - error["called"] = message + packit = MockPackitClient(expected_params, run_successfully, report_responses) + success_callback = Mock() + error_callback = Mock() mock_running_reports = MockRunningReportRepository() - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), + versions = run_reports(packit, group, disease, touchstone, MockConfig(), reports, success_callback, error_callback, mock_running_reports) @@ -282,43 +194,37 @@ def error_callback(report, message): call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), call("Success for key r2-key. New packet id is r2-version"), - call("Publishing report version r2 (r2-version)"), + call("Publishing report packet r2 (r2-version)"), call("Successfully published report packet r2 (r2-version)"), call("Success for key r1-key. New packet id is r1-version"), - call("Publishing report version r1 (r1-version)"), + call("Publishing report packet r1 (r1-version)"), call("Successfully published report packet r1 (r1-version)") ], any_order=False) mock_running_reports.assert_expected_calls() - ow.kill_report.assert_not_called() + packit.kill_task.assert_not_called() - assert success["called"] is True - assert error["called"] is False + success_callback.assert_has_calls([ + call(reports[1], "r2-version"), + call(reports[0], "r1-version") + ]) + error_callback.assert_not_called() @patch("src.utils.run_reports.logging") def test_run_reports_with_run_error(logging): run_successfully = ["r2"] report_responses = { - "r2-key": [ReportStatusResult({"status": "success", - "version": "r2-version", - "output": None})] + "r2-key": [{"status": "COMPLETE", + "packetId": "r2-version"}] } - packit = MockOrderlyWebAPI(run_successfully, report_responses, - expected_params) - wrapper = OrderlyWebClientWrapper(None, lambda x: ow) - success = {} - error = {"called": False} - - def success_callback(report, version): - success["called"] = True - - def error_callback(report, message): - error["called"] = message + packit = MockPackitClient(expected_params, run_successfully, report_responses) + success_callback = Mock() + error_callback = Mock() mock_running_reports = MockRunningReportRepository() - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), + versions = run_reports(packit, group, disease, touchstone, MockConfig(), reports, success_callback, error_callback, mock_running_reports) @@ -330,7 +236,7 @@ def error_callback(report, message): logging.info.assert_has_calls([ call(expected_run_rpt_2_log), call("Success for key r2-key. New packet id is r2-version"), - call("Publishing report version r2 (r2-version)"), + call("Publishing report packet r2 (r2-version)"), call("Successfully published report packet r2 (r2-version)") ], any_order=False) args, kwargs = logging.exception.call_args @@ -350,36 +256,27 @@ def error_callback(report, message): call(group, disease, "r2", "r2-key") ], any_order=False) - ow.kill_report.assert_not_called() + packit.kill_task.assert_not_called() - assert success["called"] is True - assert error["called"] == expected_err + success_callback.assert_called_with(reports[1], "r2-version") + error_callback.assert_called_with(reports[0], expected_err) @patch("src.utils.run_reports.logging") def test_run_reports_with_status_error(logging): run_successfully = ["r1", "r2"] report_responses = { - "r2-key": [ReportStatusResult({"status": "success", - "version": "r2-version", - "output": None})] + "r2-key": [{"status": "COMPLETE", + "packetId": "r2-version"}] } - ow = MockOrderlyWebAPI(run_successfully, report_responses, - expected_params, expected_timeouts) - wrapper = OrderlyWebClientWrapper(None, lambda x: ow) - success = {} - error = {"called": False} - - def success_callback(report, version): - success["called"] = version - - def error_callback(report, message): - error["called"] = message + packit = MockPackitClient(expected_params, run_successfully, report_responses) + success_callback = Mock() + error_callback = Mock() mock_running_reports = MockRunningReportRepository() - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), + versions = run_reports(packit, group, disease, touchstone, MockConfig(), reports, success_callback, error_callback, mock_running_reports) @@ -391,46 +288,36 @@ def error_callback(report, message): call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), call("Success for key r2-key. New packet id is r2-version"), - call("Publishing report version r2 (r2-version)"), + call("Publishing report packet r2 (r2-version)"), call("Successfully published report packet r2 (r2-version)") ], any_order=False) args, kwargs = logging.exception.call_args assert str(args[0]) == "test-status-error: r1-key" mock_running_reports.assert_expected_calls() - ow.kill_report.assert_not_called() + packit.kill_task.assert_not_called() expected_err = "test-status-error: r1-key" - assert success["called"] == "r2-version" - assert error["called"] == expected_err + success_callback.assert_called_with(reports[1], "r2-version") + error_callback.assert_called_with(reports[0], expected_err) @patch("src.utils.run_reports.logging") def test_run_reports_with_status_failure(logging): run_successfully = ["r1", "r2"] report_responses = { - "r1-key": [ReportStatusResult({"status": "success", - "version": "r1-version", - "output": None})], - "r2-key": [ReportStatusResult({"status": "error", - "version": None, - "output": None})] + "r1-key": [{"status": "COMPLETE", + "packetId": "r1-version"}], + "r2-key": [{"status": "ERROR", + "packetId": None}] } - ow = MockOrderlyWebAPI(run_successfully, report_responses, - expected_params, expected_timeouts) - wrapper = OrderlyWebClientWrapper(None, lambda x: ow) - success = {} - error = {"called": False} - - def success_callback(report, version): - success["called"] = version - - def error_callback(report, message): - error["called"] = message + packit = MockPackitClient(expected_params, run_successfully, report_responses) + success_callback = Mock() + error_callback = Mock() mock_running_reports = MockRunningReportRepository() - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), + versions = run_reports(packit, group, disease, touchstone, MockConfig(), reports, success_callback, error_callback, mock_running_reports) @@ -442,46 +329,36 @@ def error_callback(report, message): call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), call("Success for key r1-key. New packet id is r1-version"), - call("Publishing report version r1 (r1-version)"), + call("Publishing report packet r1 (r1-version)"), call("Successfully published report packet r1 (r1-version)") ], any_order=False) logging.error.assert_has_calls([ - call("Failure for key r2-key. Status: error") + call("Failure for key r2-key. Status: ERROR") ], any_order=False) mock_running_reports.assert_expected_calls() - ow.kill_report.assert_not_called() - assert success["called"] == "r1-version" - assert error["called"] == "Failure for key r2-key. Status: error" + packit.kill_task.assert_not_called() + success_callback.assert_called_with(reports[0], "r1-version") + error_callback.assert_called_with(reports[1], "Failure for key r2-key. Status: ERROR") @patch("src.utils.run_reports.logging") def test_run_reports_with_run_cancelled(logging): run_successfully = ["r1", "r2"] report_responses = { - "r1-key": [ReportStatusResult({"status": "success", - "version": "r1-version", - "output": None})], - "r2-key": [ReportStatusResult({"status": "interrupted", - "version": None, - "output": None})] + "r1-key": [{"status": "COMPLETE", + "packetId": "r1-version"}], + "r2-key": [{"status": "CANCELLED", + "packetId": None}] } - ow = MockOrderlyWebAPI(run_successfully, report_responses, - expected_params, expected_timeouts) - wrapper = OrderlyWebClientWrapper(None, lambda x: ow) - success = {} - error = {"called": False} - - def success_callback(report, version): - success["called"] = version - - def error_callback(report, message): - error["called"] = message + packit = MockPackitClient(expected_params, run_successfully, report_responses) + success_callback = Mock() + error_callback = Mock() mock_running_reports = MockRunningReportRepository() - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), + versions = run_reports(packit, group, disease, touchstone, MockConfig(), reports, success_callback, error_callback, mock_running_reports) @@ -493,47 +370,37 @@ def error_callback(report, message): call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), call("Success for key r1-key. New packet id is r1-version"), - call("Publishing report version r1 (r1-version)"), + call("Publishing report packet r1 (r1-version)"), call("Successfully published report packet r1 (r1-version)") ], any_order=False) logging.error.assert_has_calls([ - call("Failure for key r2-key. Status: interrupted") + call("Failure for key r2-key. Status: CANCELLED") ], any_order=False) mock_running_reports.assert_expected_calls() - ow.kill_report.assert_not_called() - assert success["called"] == "r1-version" + packit.kill_task.assert_not_called() + success_callback.assert_called_with(reports[0], "r1-version") # expect error callback not called for cancelled runs - assert error["called"] is False + error_callback.assert_not_called() @patch("src.utils.run_reports.logging") def test_run_reports_with_publish_failure(logging): run_successfully = ["r1", "r2"] report_responses = { - "r1-key": [ReportStatusResult({"status": "success", - "version": "r1-version", - "output": None})], - "r2-key": [ReportStatusResult({"status": "success", - "version": "r2-version", - "output": None})] + "r1-key": [{"status": "COMPLETE", + "packetId": "r1-version"}], + "r2-key": [{"status": "COMPLETE", + "packetId": "r2-version"}] } fail_publish = ["r2"] - ow = MockOrderlyWebAPI(run_successfully, report_responses, - expected_params, expected_timeouts, fail_publish) - wrapper = OrderlyWebClientWrapper(None, lambda x: ow) - success = {} - error = {} - - def success_callback(report, version): - success["called"] = version - - def error_callback(report, message): - error["called"] = message + packit = MockPackitClient(expected_params, run_successfully, report_responses, fail_publish) + success_callback = Mock() + error_callback = Mock() mock_running_reports = MockRunningReportRepository() - versions = run_reports(wrapper, group, disease, touchstone, MockConfig(), + versions = run_reports(packit, group, disease, touchstone, MockConfig(), reports, success_callback, error_callback, mock_running_reports) @@ -542,24 +409,24 @@ def error_callback(report, message): "r2-version": {"published": False, "report": "r2"} } - expected_err = "Failed to publish report version r2 (r2-version)" + expected_err = "Failed to publish report packet r2 (r2-version)" logging.info.assert_has_calls([ call(expected_run_rpt_1_log), call(expected_run_rpt_2_log), call("Success for key r1-key. New packet id is r1-version"), - call("Publishing report version r1 (r1-version)"), + call("Publishing report packet r1 (r1-version)"), call("Successfully published report packet r1 (r1-version)"), call("Success for key r2-key. New packet id is r2-version"), - call("Publishing report version r2 (r2-version)") + call("Publishing report packet r2 (r2-version)") ], any_order=False) logging.error.assert_has_calls([ call(expected_err) ], any_order=False) mock_running_reports.assert_expected_calls() - ow.kill_report.assert_not_called() - assert success["called"] == "r1-version" - assert error["called"] == expected_err + packit.kill_task.assert_not_called() + success_callback.assert_called_with(reports[0], "r1-version") + error_callback.assert_called_with(reports[1], expected_err) class MockRunningReportRepository: @@ -614,40 +481,6 @@ def publish(self, name, packit_id, roles): return True -#class MockOrderlyWebAPI: -# def __init__(self, run_successfully, report_responses, expected_params, -# expected_timeouts, fail_publish=None): -# if fail_publish is None: -# fail_publish = [] -# self.run_successfully = run_successfully -# self.report_responses = report_responses -# self.expected_params = expected_params -# self.expected_timeouts = expected_timeouts -# self.fail_publish = fail_publish -# self.kill_report = Mock() - -# def run_report(self, name, params, timeout): -# assert params == self.expected_params[name] -# assert timeout == self.expected_timeouts[name] -# if name in self.run_successfully: -# return name + "-key" -# else: -# raise Exception("test-run-error: " + name)# - -# def report_status(self, key): -# if key in self.report_responses and \ -# len(self.report_responses[key]) > 0: -# return self.report_responses[key].pop(0) -# else: -# raise Exception("test-status-error: " + key) - -# def publish_report(self, name, version): -# if name in self.fail_publish: -# raise Exception("Publish failed") -# else: -# return True - - PACKIT_URL = "http://test-packit" class MockConfig: From f410ce6a50a64ac5696d5d9ca5c7143f05d9831b Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 15 Jul 2025 17:37:47 +0100 Subject: [PATCH 17/36] fix send diagnostic email unit tests --- test/unit/test_send_diagnostic_report_email.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/test_send_diagnostic_report_email.py b/test/unit/test_send_diagnostic_report_email.py index 1e0b250..cb04d08 100644 --- a/test/unit/test_send_diagnostic_report_email.py +++ b/test/unit/test_send_diagnostic_report_email.py @@ -38,7 +38,7 @@ def test_url_encodes_url_in_email(send_email): "2020-11-04T12:22:53", "no_vaccination", mock_config) - url = "http://orderly-web/report/{}/1234-abcd/".format(encoded, encoded) + url = "http://test-packit/{}/1234-abcd/".format(encoded) send_email.assert_has_calls([ call(fake_emailer, report, @@ -64,7 +64,7 @@ def test_additional_recipients_used(send_email): "no_vaccination", mock_config, "someone@example.com") - url = "http://orderly-web/report/{}/1234-abcd/".format(name) + url = "http://test-packit/{}/1234-abcd/".format(name) send_email.assert_has_calls([ call(fake_emailer, report, @@ -90,7 +90,7 @@ def test_additional_recipients_not_used(send_email): "no_vaccination", mock_config, "someone@example.com") - url = "http://orderly-web/report/{}/1234-abcd/".format(name) + url = "http://test-packit/{}/1234-abcd/".format(name) send_email.assert_has_calls([ call(fake_emailer, report, From cbe6a1d47473e7308135407528b9413e6858efd4 Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 16 Jul 2025 10:40:07 +0100 Subject: [PATCH 18/36] refresh git before attempting to get latest commit or run reports --- src/packit_client.py | 6 +++++- src/utils/run_reports.py | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/packit_client.py b/src/packit_client.py index 10fd61c..2def03c 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -18,7 +18,7 @@ def __url(self, relative_url): def handle_response(response): if response.status_code != 200 and response.status_code != 204: raise PackitClientException(response) - return response.json() + return response.json() if len(response.text) > 0 else None @staticmethod def serialize(data): @@ -89,6 +89,10 @@ def __wait_for_packet_to_be_imported(self, packet_id): time.sleep(poll_seconds) raise Exception(f"Packet {packet_id} was not imported into Packit after {seconds_max}s") + def refresh_git(self): + def do_refresh_git(): + self.__post("/runner/git/fetch", None) + self.__execute(do_refresh_git) def run(self, packet_group, parameters): def do_run(): diff --git a/src/utils/run_reports.py b/src/utils/run_reports.py index 7a80b07..875d032 100644 --- a/src/utils/run_reports.py +++ b/src/utils/run_reports.py @@ -36,6 +36,16 @@ def run_reports(packit, group, disease, touchstone, config, reports, logging.error(error) return new_packets + try: + packit.refresh_git() + except Exception as ex: + error = "Failed to refresh git; could not begin task" + for report in reports: + error_callback(report, error) + logging.error(error) + return new_packets + + # Start configured reports for report in reports: # Kill any currently running task for this group/disease/report From 45849ebb525a7f4fe4f0ea1cd0edfc37faf03147 Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 16 Jul 2025 15:59:30 +0100 Subject: [PATCH 19/36] more test fixes --- src/task_run_diagnostic_reports.py | 14 +++++++++---- test/integration/test_config.py | 4 ++-- .../test_task_run_diagnostic_reports.py | 20 +++++++++---------- test/unit/test_packit_client.py | 10 ++++++++++ test/unit/test_run_reports.py | 3 +++ 5 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/task_run_diagnostic_reports.py b/src/task_run_diagnostic_reports.py index bbfdcbe..4ad36f1 100644 --- a/src/task_run_diagnostic_reports.py +++ b/src/task_run_diagnostic_reports.py @@ -116,16 +116,22 @@ def create_ticket(group, disease, touchstone, scenario, logging.exception(ex) +def create_tag(yt, tag_name): + try: + yt.create_tag(tag_name) + except YTException: + logging.error(f"Failed to create YouTrack tag {tag_name}") + def create_tags(yt, group, disease, touchstone, report): tags = yt.get_tags(fields=["name"]) if len([t for t in tags if t["name"] == disease]) == 0: - yt.create_tag(disease) + create_tag(yt, disease) if len([t for t in tags if t["name"] == group]) == 0: - yt.create_tag(group) + create_tag(yt, group) if len([t for t in tags if t["name"] == touchstone]) == 0: - yt.create_tag(touchstone) + create_tag(yt, touchstone) if len([t for t in tags if t["name"] == report.name]) == 0: - yt.create_tag(report.name) + create_tag(yt, report.name) def send_diagnostic_report_email(emailer, diff --git a/test/integration/test_config.py b/test/integration/test_config.py index 6efaeb9..e742635 100644 --- a/test/integration/test_config.py +++ b/test/integration/test_config.py @@ -19,8 +19,8 @@ def test_montagu_password(): assert config.montagu_password == "password" -def test_orderlyweb_url(): - assert config.orderlyweb_url == "http://localhost:8888" +def test_packit_url(): + assert config.packit_url == "https://localhost" def test_youtrack_token(): diff --git a/test/integration/test_task_run_diagnostic_reports.py b/test/integration/test_task_run_diagnostic_reports.py index 4c4a2f4..fa1b05d 100644 --- a/test/integration/test_task_run_diagnostic_reports.py +++ b/test/integration/test_task_run_diagnostic_reports.py @@ -178,7 +178,7 @@ def test_ticket_created_on_success(): "Check & share diag report with testGroup (testDisease) {}" \ .format(yt.test_touchstone) expected_desc1 = "Report run triggered by upload to scenario: s1. " \ - "http://localhost:8888/report/{}/{}/".format(r1, v1) + "https://localhost/packit/{}/{}/".format(r1, v1) assert i1["summary"] == expected_summary assert i1["description"] == expected_desc1 assignee = get_field(i1, "Assignee") @@ -194,7 +194,7 @@ def test_ticket_created_on_success(): assert yt.test_touchstone in tags expected_desc2 = "Report run triggered by upload to scenario: s1. " \ - "http://localhost:8888/report/{}/{}/".format(r2, v2) + "https://localhost/packit/{}/{}/".format(r2, v2) assert i2["summary"] == expected_summary assert i2["description"] == expected_desc2 assignee = get_field(i2, "Assignee") @@ -210,9 +210,9 @@ def test_ticket_created_on_success(): assert yt.test_touchstone in tags -@mock.patch('src.config.Config.orderlyweb_url', new_callable=PropertyMock) -def test_ticket_created_on_error(mock_orderlyweb_url): - mock_orderlyweb_url.return_value = "http://bad-url" +@mock.patch('src.config.Config.packit_url', new_callable=PropertyMock) +def test_ticket_created_on_error(mock_packit_url): + mock_packit_url.return_value = "http://bad-url" result = run_diagnostic_reports("testGroup", "testDisease", yt.test_touchstone, @@ -235,7 +235,7 @@ def test_ticket_created_on_error(mock_orderlyweb_url): expected_err = "Report run triggered by upload to scenario: s1. " \ "Auto-run failed with error: " + \ - "Orderlyweb authentication failed; could not begin task" + "Packit authentication failed; could not begin task" i1 = issues[0] i2 = issues[1] @@ -288,12 +288,12 @@ def test_ticket_update_on_success(): "Check & share diag report with testGroup (testDisease) {}" \ .format(yt.test_touchstone) expected_desc1 = "Report run triggered by upload to scenario: s1. " \ - "http://localhost:8888/report/{}/{}/".format(r1, v1) + "https://localhost/packit/{}/{}/".format(r1, v1) assert i1["summary"] == expected_summary assert i1["description"] == expected_desc1 expected_desc2 = "Report run triggered by upload to scenario: s1. " \ - "http://localhost:8888/report/{}/{}/".format(r2, v2) + "https://localhost/packit/{}/{}/".format(r2, v2) assert i2["summary"] == expected_summary assert i2["description"] == expected_desc2 @@ -325,11 +325,11 @@ def test_ticket_update_on_success(): "Check & share diag report with testGroup (testDisease) {}" \ .format(yt.test_touchstone) expected_desc1 = "Report run triggered by upload to scenario: s1. " \ - "http://localhost:8888/report/{}/{}/".format(r1, v1) + "https://localhost/packit/{}/{}/".format(r1, v1) assert i1["summary"] == expected_summary assert i1["description"] == expected_desc1 expected_desc2 = "Report run triggered by upload to scenario: s1. " \ - "http://localhost:8888/report/{}/{}/".format(r2, v2) + "https://localhost/packit/{}/{}/".format(r2, v2) assert i2["summary"] == expected_summary assert i2["description"] == expected_desc2 diff --git a/test/unit/test_packit_client.py b/test/unit/test_packit_client.py index 959abff..3b32d12 100644 --- a/test/unit/test_packit_client.py +++ b/test/unit/test_packit_client.py @@ -41,6 +41,16 @@ def test_authenticates_on_init(MockMontaguAPI, requests_mock): assert sut.token == "test-packit-token" +@patch("montagu.MontaguAPI") +def test_refresh_git(MockMontaguAPI, requests_mock): + mock_auth(MockMontaguAPI, requests_mock) + requests_mock.post(f"{PACKIT_URL}/api/runner/git/fetch", text = None) + + sut = PackitClient(config) + sut.refresh_git() + + assert_expected_packit_api_request(requests_mock, 1, "POST", f"{PACKIT_URL}/api/runner/git/fetch") + @patch("montagu.MontaguAPI") def test_run(MockMontaguAPI, requests_mock): mock_auth(MockMontaguAPI, requests_mock) diff --git a/test/unit/test_run_reports.py b/test/unit/test_run_reports.py index 9bf300a..76bc061 100644 --- a/test/unit/test_run_reports.py +++ b/test/unit/test_run_reports.py @@ -48,6 +48,8 @@ def test_run_reports(logging): reports, success_callback, error_callback, mock_running_reports) + packit.refresh_git.assert_called_with() + assert versions == { "r1-version": {"published": True, "report": "r1"}, "r2-version": {"published": True, "report": "r2"} @@ -459,6 +461,7 @@ def __init__(self, expected_params, run_successfully, report_responses, fail_pub self.report_responses = report_responses self.fail_publish = fail_publish self.kill_task = Mock() + self.refresh_git = Mock() def run(self, name, params): assert params == self.expected_params[name] From 38ed60b74ef1d96132856c0818a0bb28cb2094d7 Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 16 Jul 2025 17:13:12 +0100 Subject: [PATCH 20/36] integration test fixes --- config/config.yml | 2 -- src/config.py | 2 +- src/packit_client.py | 2 +- test/integration/test_config.py | 11 ++++++----- test/integration/test_run_reports.py | 6 +++--- test/unit/test_run_reports.py | 2 +- 6 files changed, 12 insertions(+), 13 deletions(-) diff --git a/config/config.yml b/config/config.yml index 9a0a4e4..47eef76 100644 --- a/config/config.yml +++ b/config/config.yml @@ -28,7 +28,6 @@ tasks: - minimal_modeller@example.com - science@example.com subject: "VIMC diagnostic report: {touchstone} - {group} - {disease}" - timeout: 300 assignee: a.hill publish_roles: - minimal.modeller @@ -43,7 +42,6 @@ tasks: - other_modeller@example.com - science@example.com subject: "New version of another Orderly report" - timeout: 1200 assignee: e.russell publish_roles: - other.modeller diff --git a/src/config.py b/src/config.py index c57f6e2..56da3f1 100644 --- a/src/config.py +++ b/src/config.py @@ -50,7 +50,7 @@ def packit_url(self): return self.__server("packit")["url"] @property - def disable_certificate_verify(self): + def packit_disable_certificate_verify(self): return self.__server("packit")["disable_certificate_verify"] @property diff --git a/src/packit_client.py b/src/packit_client.py index 2def03c..bae677a 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -8,7 +8,7 @@ class PackitClient: def __init__(self, config): self.__config = config - self.__verify = not config.disable_certificate_verify + self.__verify = not config.packit_disable_certificate_verify self.__authenticate() def __url(self, relative_url): diff --git a/test/integration/test_config.py b/test/integration/test_config.py index e742635..a7a136e 100644 --- a/test/integration/test_config.py +++ b/test/integration/test_config.py @@ -20,8 +20,10 @@ def test_montagu_password(): def test_packit_url(): - assert config.packit_url == "https://localhost" + assert config.packit_url == "https://localhost/packit" +def test_packit_disable_certificate_verify(): + assert config.packit_disable_certificate_verify == True def test_youtrack_token(): assert config.youtrack_token == "None" @@ -61,16 +63,15 @@ def test_diagnostic_reports(): ["minimal_modeller@example.com", "science@example.com"] assert reports[0].success_email_subject == \ "VIMC diagnostic report: {touchstone} - {group} - {disease}" - assert reports[0].timeout == 300 + assert reports[0].publish_roles == ["minimal.modeller", "Funders"] assert reports[1].name == "diagnostic-param" - assert len(reports[1].parameters.keys()) == 1 - assert reports[1].parameters["nmin"] == 0 + assert reports[1].parameters == { "a": 1, "b": 2, "c": 3 } assert reports[1].success_email_recipients == \ ["other_modeller@example.com", "science@example.com"] assert reports[1].success_email_subject == \ "New version of another Orderly report" - assert reports[1].timeout == 1200 + assert reports[1].publish_roles == ["other.modeller"] def test_diagnostic_reports_nonexistent(): diff --git a/test/integration/test_run_reports.py b/test/integration/test_run_reports.py index 773f613..0a714a9 100644 --- a/test/integration/test_run_reports.py +++ b/test/integration/test_run_reports.py @@ -1,5 +1,5 @@ from src.config import Config, ReportConfig -from src.orderlyweb_client_wrapper import OrderlyWebClientWrapper +from src.packit_client import PackitClient from src.utils.run_reports import run_reports from src.utils.running_reports_repository import RunningReportsRepository @@ -11,7 +11,7 @@ def test_run_reports_handles_error(): ReportConfig("diagnostic", {}, ["test2@test.com"], "subject2", "a.ssignee", ["Funders"])] config = Config() - wrapper = OrderlyWebClientWrapper(config) + packit = PackitClient(config) success = {} error = {} @@ -23,7 +23,7 @@ def error_callback(report, message): running_reports_repository = RunningReportsRepository() - versions = run_reports(wrapper, "testGroup", "testDisease", + versions = run_reports(packit, "testGroup", "testDisease", "testTouchstone", config, reports, success_callback, error_callback, running_reports_repository) diff --git a/test/unit/test_run_reports.py b/test/unit/test_run_reports.py index 76bc061..7a634f4 100644 --- a/test/unit/test_run_reports.py +++ b/test/unit/test_run_reports.py @@ -524,7 +524,7 @@ def packit_url(self): return PACKIT_URL @property - def disable_certificate_verify(self): + def packit_disable_certificate_verify(self): return False @property From d51373311c3d6d0b4dec84053bd8d6bce71bc9e1 Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 16 Jul 2025 17:17:51 +0100 Subject: [PATCH 21/36] update docker config --- config/docker_config.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/config/docker_config.yml b/config/docker_config.yml index 63259b8..85d9693 100644 --- a/config/docker_config.yml +++ b/config/docker_config.yml @@ -1,15 +1,16 @@ host: montagu-mq-1 servers: montagu: - url: http://montagu_api_1:8080 + url: http://montagu-api-1:8080 user: test.user@example.com password: password - orderlyweb: - url: http://montagu_orderly_web:8888 + packit: + url: http://montagu-packit-api:8080 + disable_certificate_verify: true youtrack: token: smtp: - host: montagu_smtp_server_1 + host: montagu-smtp_server-1 port: 1025 from: noreply@example.com tasks: From 06a27b8fd5a3290cbba925be53218a5b2c5153d7 Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 16 Jul 2025 23:37:26 +0100 Subject: [PATCH 22/36] lint --- .github/workflows/test-and-push.yml | 2 +- src/packit_client.py | 79 ++++++---- src/packit_client_exception.py | 5 +- src/task_run_diagnostic_reports.py | 1 + src/utils/run_reports.py | 27 +++- test/integration/test_config.py | 8 +- .../test_task_run_diagnostic_reports.py | 12 +- test/unit/test_create_ticket.py | 4 +- test/unit/test_packit_client.py | 140 ++++++++++++------ test/unit/test_run_reports.py | 43 ++++-- 10 files changed, 210 insertions(+), 111 deletions(-) diff --git a/.github/workflows/test-and-push.yml b/.github/workflows/test-and-push.yml index 6a9f034..3c4a410 100644 --- a/.github/workflows/test-and-push.yml +++ b/.github/workflows/test-and-push.yml @@ -15,7 +15,7 @@ jobs: - name: Install dependencies run: | - pip3 install pytest-cov pycodestyle codecov + pip3 install pytest-cov pycodestyle codecov hatch pip3 install -r requirements.txt - name: Run dependencies diff --git a/src/packit_client.py b/src/packit_client.py index bae677a..332e772 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -5,6 +5,7 @@ import time from .packit_client_exception import PackitClientException + class PackitClient: def __init__(self, config): self.__config = config @@ -24,25 +25,36 @@ def handle_response(response): def serialize(data): return None if data is None else json.dumps(data) - def __get(self, relative_url, headers = None): + def __get(self, relative_url, headers=None): if headers is None: headers = self.__default_headers - response = requests.get(self.__url(relative_url), headers=headers, verify = self.__verify) + response = requests.get(self.__url(relative_url), + headers=headers, + verify=self.__verify) return PackitClient.handle_response(response) def __post(self, relative_url, data): - response = requests.post(self.__url(relative_url), data=PackitClient.serialize(data), headers=self.__default_headers, verify = self.__verify) + response = requests.post(self.__url(relative_url), + data=PackitClient.serialize(data), + headers=self.__default_headers, + verify=self.__verify) return PackitClient.handle_response(response) def __put(self, relative_url, data): - response = requests.put(self.__url(relative_url), data=PackitClient.serialize(data), headers=self.__default_headers, verify = self.__verify) + response = requests.put(self.__url(relative_url), + data=PackitClient.serialize(data), + headers=self.__default_headers, + verify=self.__verify) return PackitClient.handle_response(response) def __authenticate(self): try: - monty = montagu.MontaguAPI(self.__config.montagu_url, self.__config.montagu_user, - self.__config.montagu_password) - packit_login_response = self.__get("/auth/login/montagu", {"Authorization": f"Bearer {monty.token}"}) + monty = montagu.MontaguAPI(self.__config.montagu_url, + self.__config.montagu_user, + self.__config.montagu_password) + packit_login_response = self.__get( + "/auth/login/montagu", + {"Authorization": f"Bearer {monty.token}"}) self.auth_success = True self.token = packit_login_response["token"] self.__default_headers = { @@ -53,22 +65,24 @@ def __authenticate(self): self.auth_success = False logging.exception(ex) - def __execute(self, func, *args): # TODO: maybe don't need args? - # retry an operation if it fails auth (probably because of an expired packit token) - try: + def __execute(self, func, *args): # TODO: maybe don't need args? + # retry an operation if it fails auth (probably because of an expired + # packit token) + try: + return func(*args) + except PackitClientException as ex: + if ex.response.status_code == 401: + self.__authenticate() return func(*args) - except PackitClientException as ex: - if ex.response.status_code == 401: - self.__authenticate() - return func(*args) - else: - raise ex + else: + raise ex def __get_latest_commit_for_branch(self, branch): branches_response = self.__get("/runner/git/branches") branches = branches_response["branches"] - branch_details = next(filter(lambda b: b["name"] == branch, branches), None) - if branch_details == None: + branch_details = next( + filter(lambda b: b["name"] == branch, branches), None) + if branch_details is None: raise Exception(f"Git details not found for branch {branch}") return branch_details["commitHash"] @@ -87,7 +101,9 @@ def __wait_for_packet_to_be_imported(self, packet_id): logging.info(f"Waiting for packet {packet_id}...") poll_counter = poll_counter + 1 time.sleep(poll_seconds) - raise Exception(f"Packet {packet_id} was not imported into Packit after {seconds_max}s") + raise Exception( + f"Packet {packet_id} was not imported into Packit after " + + f"{seconds_max}s") def refresh_git(self): def do_refresh_git(): @@ -119,29 +135,30 @@ def do_kill_task(): return self.__post(f"/runner/cancel/{task_id}", None) return self.__execute(do_kill_task) - def publish(self, name, packet_id, roles): - # mimic OW publishing by setting packet-level permission for a new report packet permission - # on a list of configured roles. NB: These role can either be user roles or groups. If users, - # these need to be user names not email addresses. + # mimic OW publishing by setting packet-level permission for a new + # report packet permission on a list of configured roles. + # NB: These role can either be user # roles or groups. + # If users, these need to be user names not email addresses. def do_publish_to_role(role): data = { - "addPermissions": [{ - "permission": "packet.read", - "packetId": packet_id - }], - "removePermissions": [] + "addPermissions": [{ + "permission": "packet.read", + "packetId": packet_id + }], + "removePermissions": [] } self.__put(f"/roles/{role}/permissions", data) - self.__execute(lambda: self.__wait_for_packet_to_be_imported(packet_id)) + self.__execute( + lambda: self.__wait_for_packet_to_be_imported(packet_id)) logging.info(f"Publishing packet {name}({packet_id})") success = True for role in roles: try: - logging.info(f"...to role {role}") - self.__execute(lambda: do_publish_to_role(role)) + logging.info(f"...to role {role}") + self.__execute(lambda: do_publish_to_role(role)) except Exception as ex: logging.exception(ex) success = False diff --git a/src/packit_client_exception.py b/src/packit_client_exception.py index 3485e13..280b917 100644 --- a/src/packit_client_exception.py +++ b/src/packit_client_exception.py @@ -2,7 +2,8 @@ class PackitClientException(Exception): def __init__(self, response): self.response = response json = response.json() - msg = f"Unexpected response status from Packit API: {response.status_code}." + msg = "Unexpected response status from Packit API: " + + f"{response.status_code}." if "error" in json and "detail" in json["error"]: msg = f"{msg} Detail: {json["error"]["detail"]}" - super().__init__(msg) \ No newline at end of file + super().__init__(msg) diff --git a/src/task_run_diagnostic_reports.py b/src/task_run_diagnostic_reports.py index 4ad36f1..a8b7346 100644 --- a/src/task_run_diagnostic_reports.py +++ b/src/task_run_diagnostic_reports.py @@ -122,6 +122,7 @@ def create_tag(yt, tag_name): except YTException: logging.error(f"Failed to create YouTrack tag {tag_name}") + def create_tags(yt, group, disease, touchstone, report): tags = yt.get_tags(fields=["name"]) if len([t for t in tags if t["name"] == disease]) == 0: diff --git a/src/utils/run_reports.py b/src/utils/run_reports.py index 875d032..70a37ca 100644 --- a/src/utils/run_reports.py +++ b/src/utils/run_reports.py @@ -6,12 +6,14 @@ TASK_STATUS_COMPLETE = "COMPLETE" TASK_STATUS_CANCELLED = "CANCELLED" + def task_is_finished(poll_response): status = poll_response["status"] return status not in [ TASK_STATUS_PENDING, TASK_STATUS_RUNNING - ] + ] + def publish_report(packit, name, packet_id, roles): try: @@ -24,6 +26,7 @@ def publish_report(packit, name, packet_id, roles): def params_to_string(params): return ", ".join([f"{key}={value}" for key, value in params.items()]) + def run_reports(packit, group, disease, touchstone, config, reports, success_callback, error_callback, running_reports_repo): running_reports = {} @@ -45,7 +48,6 @@ def run_reports(packit, group, disease, touchstone, config, reports, logging.error(error) return new_packets - # Start configured reports for report in reports: # Kill any currently running task for this group/disease/report @@ -97,17 +99,26 @@ def run_reports(packit, group, disease, touchstone, config, reports, packet_id = result["packetId"] name = report.name - report_config = next(filter(lambda report: report.name == name, reports), None) + report_config = next( + filter(lambda report: report.name == name, + reports), + None) if report_config is not None: - logging.info("Publishing report packet {} ({})".format(name, packet_id)) - published = publish_report(packit, name, packet_id, report_config.publish_roles) + logging.info( + "Publishing report packet {} ({})" + .format(name, packet_id)) + published = publish_report( + packit, name, packet_id, + report_config.publish_roles) if published: logging.info( - f"Successfully published report packet {name} ({packet_id})" - ) + "Successfully published report packet " + + f"{name} ({packet_id})" + ) success_callback(report, packet_id) else: - error = f"Failed to publish report packet {name} ({packet_id})" + error = "Failed to publish report packet " + + f"{name} ({packet_id})" logging.error(error) error_callback(report, error) new_packets[packet_id] = { diff --git a/test/integration/test_config.py b/test/integration/test_config.py index a7a136e..b52c230 100644 --- a/test/integration/test_config.py +++ b/test/integration/test_config.py @@ -22,8 +22,10 @@ def test_montagu_password(): def test_packit_url(): assert config.packit_url == "https://localhost/packit" + def test_packit_disable_certificate_verify(): - assert config.packit_disable_certificate_verify == True + assert config.packit_disable_certificate_verify + def test_youtrack_token(): assert config.youtrack_token == "None" @@ -60,13 +62,13 @@ def test_diagnostic_reports(): assert reports[0].name == "diagnostic" assert len(reports[0].parameters.keys()) == 0 assert reports[0].success_email_recipients == \ - ["minimal_modeller@example.com", "science@example.com"] + ["minimal_modeller@example.com", "science@example.com"] assert reports[0].success_email_subject == \ "VIMC diagnostic report: {touchstone} - {group} - {disease}" assert reports[0].publish_roles == ["minimal.modeller", "Funders"] assert reports[1].name == "diagnostic-param" - assert reports[1].parameters == { "a": 1, "b": 2, "c": 3 } + assert reports[1].parameters == {"a": 1, "b": 2, "c": 3} assert reports[1].success_email_recipients == \ ["other_modeller@example.com", "science@example.com"] assert reports[1].success_email_subject == \ diff --git a/test/integration/test_task_run_diagnostic_reports.py b/test/integration/test_task_run_diagnostic_reports.py index fa1b05d..3509425 100644 --- a/test/integration/test_task_run_diagnostic_reports.py +++ b/test/integration/test_task_run_diagnostic_reports.py @@ -103,13 +103,13 @@ def test_run_diagnostic_reports(): diagnostic_is_first = result[versions[0]]["report"] == "diagnostic" if diagnostic_is_first: - report_1 = "diagnostic" - report_2 = "diagnostic-param" - email_props = [diagnostic_email_props, diagnostic_param_email_props] + report_1 = "diagnostic" + report_2 = "diagnostic-param" + email_props = [diagnostic_email_props, diagnostic_param_email_props] else: - report_1 = "diagnostic-param" - report_2 = "diagnostic" - email_props = [diagnostic_param_email_props, diagnostic_email_props] + report_1 = "diagnostic-param" + report_2 = "diagnostic" + email_props = [diagnostic_param_email_props, diagnostic_email_props] url_template = "https://localhost/packit/{}/{}/" url_1 = url_template.format(report_1, versions[0]) diff --git a/test/unit/test_create_ticket.py b/test/unit/test_create_ticket.py index d03a66d..509d6cf 100644 --- a/test/unit/test_create_ticket.py +++ b/test/unit/test_create_ticket.py @@ -87,7 +87,7 @@ def test_create_ticket_without_version(): @patch("src.task_run_diagnostic_reports.logging") def test_create_ticket_logs_errors(logging): report = ReportConfig("TEST", {}, ["to@example.com"], - "Hi", "a.ssignee", ["Funders"]) + "Hi", "a.ssignee", ["Funders"]) mock_config: Config = MockConfig() mock_client = Mock(spec=YTClient("", "")) mock_client.get_issues = Mock(return_value=[]) @@ -101,7 +101,7 @@ def test_create_ticket_logs_errors(logging): def test_update_ticket(): report = ReportConfig("TEST", {}, ["to@example.com"], - "Hi", "a.ssignee", ["Funders"]) + "Hi", "a.ssignee", ["Funders"]) mock_config: Config = MockConfig() mock_client = Mock(spec=YTClient("", "")) mock_client.get_issues = Mock(return_value=["ISSUE"]) diff --git a/test/unit/test_packit_client.py b/test/unit/test_packit_client.py index 3b32d12..fddeccd 100644 --- a/test/unit/test_packit_client.py +++ b/test/unit/test_packit_client.py @@ -9,18 +9,27 @@ config = MockConfig() + def mock_auth(mock_montagu_api_class, requests_mock): mock_montagu_api = mock_montagu_api_class.return_value mock_montagu_api.token = "test-montagu-token" - requests_mock.get(f"{PACKIT_URL}/api/auth/login/montagu", text = json.dumps({"token": "test-packit-token"} )) + requests_mock.get(f"{PACKIT_URL}/api/auth/login/montagu", + text=json.dumps({"token": "test-packit-token"})) + -def assert_expected_packit_api_request(requests_mock, index, method, url, text = None): +def assert_expected_packit_api_request( + requests_mock, + index, + method, + url, + text=None): req = requests_mock.request_history[index] assert req.headers["Authorization"] == "Bearer test-packit-token" assert req.method == method assert req.url == url assert req.text == text + def assert_expected_packit_auth_request(requests_mock, index): auth_req = requests_mock.request_history[index] assert auth_req.method == "GET" @@ -34,7 +43,8 @@ def test_authenticates_on_init(MockMontaguAPI, requests_mock): sut = PackitClient(config) - MockMontaguAPI.assert_called_with("http://test-montagu", "test.montagu.user", "montagu_password") + MockMontaguAPI.assert_called_with( + "http://test-montagu", "test.montagu.user", "montagu_password") auth_call = requests_mock.request_history[0] assert_expected_packit_auth_request(requests_mock, 0) assert sut.auth_success @@ -44,98 +54,136 @@ def test_authenticates_on_init(MockMontaguAPI, requests_mock): @patch("montagu.MontaguAPI") def test_refresh_git(MockMontaguAPI, requests_mock): mock_auth(MockMontaguAPI, requests_mock) - requests_mock.post(f"{PACKIT_URL}/api/runner/git/fetch", text = None) + requests_mock.post(f"{PACKIT_URL}/api/runner/git/fetch", text=None) sut = PackitClient(config) sut.refresh_git() - assert_expected_packit_api_request(requests_mock, 1, "POST", f"{PACKIT_URL}/api/runner/git/fetch") + assert_expected_packit_api_request( + requests_mock, 1, "POST", f"{PACKIT_URL}/api/runner/git/fetch") + @patch("montagu.MontaguAPI") def test_run(MockMontaguAPI, requests_mock): mock_auth(MockMontaguAPI, requests_mock) - mock_branches_response = { "branches": [ - { "name": "some_other_branch", "commitHash": "xyz987" }, - { "name": "main", "commitHash": "abc123" } - ] } - requests_mock.get(f"{PACKIT_URL}/api/runner/git/branches", text = json.dumps(mock_branches_response)) - requests_mock.post(f"{PACKIT_URL}/api/runner/run", text = json.dumps({ "taskId": "test-task-id" })) + mock_branches_response = {"branches": [ + {"name": "some_other_branch", "commitHash": "xyz987"}, + {"name": "main", "commitHash": "abc123"} + ]} + requests_mock.get(f"{PACKIT_URL}/api/runner/git/branches", + text=json.dumps(mock_branches_response)) + requests_mock.post(f"{PACKIT_URL}/api/runner/run", + text=json.dumps({"taskId": "test-task-id"})) sut = PackitClient(config) task_id = sut.run("test-packet-group", {"a": 1, "b": 2}) assert task_id == "test-task-id" - assert_expected_packit_api_request(requests_mock, 1, "GET", f"{PACKIT_URL}/api/runner/git/branches") + assert_expected_packit_api_request( + requests_mock, 1, "GET", f"{PACKIT_URL}/api/runner/git/branches") expected_run_payload = json.dumps({ "name": "test-packet-group", "parameters": {"a": 1, "b": 2}, "branch": "main", "hash": "abc123" }) - assert_expected_packit_api_request(requests_mock, 2, "POST", f"{PACKIT_URL}/api/runner/run", expected_run_payload) + assert_expected_packit_api_request( + requests_mock, 2, "POST", + f"{PACKIT_URL}/api/runner/run", expected_run_payload) @patch("montagu.MontaguAPI") def test_poll_status(MockMontaguAPI, requests_mock): mock_auth(MockMontaguAPI, requests_mock) - mock_poll_response = { "status": "RUNNING" } - requests_mock.get(f"{PACKIT_URL}/api/runner/status/test-task-id", text = json.dumps(mock_poll_response)) + mock_poll_response = {"status": "RUNNING"} + requests_mock.get(f"{PACKIT_URL}/api/runner/status/test-task-id", + text=json.dumps(mock_poll_response)) sut = PackitClient(config) resp = sut.poll("test-task-id") assert resp == mock_poll_response + @patch("montagu.MontaguAPI") def test_kill_task(MockMontaguAPI, requests_mock): mock_auth(MockMontaguAPI, requests_mock) - mock_kill_response = { "status": "dead" } - requests_mock.post(f"{PACKIT_URL}/api/runner/cancel/test-task-id", text = json.dumps(mock_kill_response)) + mock_kill_response = {"status": "dead"} + requests_mock.post(f"{PACKIT_URL}/api/runner/cancel/test-task-id", + text=json.dumps(mock_kill_response)) sut = PackitClient(config) resp = sut.kill_task("test-task-id") assert resp == mock_kill_response - assert_expected_packit_api_request(requests_mock, 1, "POST", f"{PACKIT_URL}/api/runner/cancel/test-task-id") + assert_expected_packit_api_request( + requests_mock, 1, "POST", + f"{PACKIT_URL}/api/runner/cancel/test-task-id") @patch("montagu.MontaguAPI") def test_publish(MockMontaguAPI, requests_mock): mock_auth(MockMontaguAPI, requests_mock) - # publish polls for the packit - return 404 on first poll, 200 on second poll + # publish polls for the packit - return 404 on first poll, + # 200 on second poll requests_mock.get(f"{PACKIT_URL}/api/packets/test-packet-id", [ - { "text": json.dumps({ "error": { "detail": "not found" } }), "status_code": 404}, - { "text": json.dumps({ "id": "test-packet-id", "name": "A packet" }), "status_code": 200 } + {"text": json.dumps({"error": {"detail": "not found"}}), + "status_code": 404}, + {"text": json.dumps( + {"id": "test-packet-id", "name": "A packet"}), "status_code": 200} ]) expected_permissions_payload = json.dumps({ - "addPermissions": [{ "permission": "packet.read", "packetId": "test-packet-id" }], + "addPermissions": [ + {"permission": "packet.read", "packetId": "test-packet-id"} + ], "removePermissions": [] }) - requests_mock.put(f"{PACKIT_URL}/api/roles/test-role-1/permissions", json = "null") - requests_mock.put(f"{PACKIT_URL}/api/roles/test-role-2/permissions", text = "null") + requests_mock.put( + f"{PACKIT_URL}/api/roles/test-role-1/permissions", json="null") + requests_mock.put( + f"{PACKIT_URL}/api/roles/test-role-2/permissions", text="null") sut = PackitClient(config) - result = sut.publish("A packet", "test-packet-id", ["test-role-1", "test-role-2"]) - assert_expected_packit_api_request(requests_mock, 1, "GET", f"{PACKIT_URL}/api/packets/test-packet-id", None) - assert_expected_packit_api_request(requests_mock, 2, "GET", f"{PACKIT_URL}/api/packets/test-packet-id", None) - - assert_expected_packit_api_request(requests_mock, 3, "PUT", f"{PACKIT_URL}/api/roles/test-role-1/permissions", expected_permissions_payload) - assert_expected_packit_api_request(requests_mock, 4, "PUT", f"{PACKIT_URL}/api/roles/test-role-2/permissions", expected_permissions_payload) + result = sut.publish("A packet", "test-packet-id", + ["test-role-1", "test-role-2"]) + assert_expected_packit_api_request( + requests_mock, 1, "GET", + f"{PACKIT_URL}/api/packets/test-packet-id", + None) + assert_expected_packit_api_request( + requests_mock, 2, "GET", + f"{PACKIT_URL}/api/packets/test-packet-id", + None) + + assert_expected_packit_api_request( + requests_mock, 3, "PUT", + f"{PACKIT_URL}/api/roles/test-role-1/permissions", + expected_permissions_payload) + assert_expected_packit_api_request( + requests_mock, 4, "PUT", + f"{PACKIT_URL}/api/roles/test-role-2/permissions", + expected_permissions_payload) assert result + @patch("montagu.MontaguAPI") -def test_sets_auth_success_to_false_when_auth_fails(MockMontaguAPI, requests_mock): - requests_mock.get(f"{PACKIT_URL}/api/auth/login/montagu", status_code = 401, text = json.dumps({"error": "Unauthorized"})) +def test_sets_auth_success_to_false_when_auth_fails( + MockMontaguAPI, + requests_mock): + requests_mock.get(f"{PACKIT_URL}/api/auth/login/montagu", + status_code=401, + text=json.dumps({"error": "Unauthorized"})) sut = PackitClient(config) assert not sut.auth_success + @patch("montagu.MontaguAPI") def test_reauthenticates_on_401(MockMontaguAPI, requests_mock): # Reauthentication should take place as part of the __execute wrapper used - # with all methods which require authentication - here we just test a sample - # method to check the pattern works. + # with all methods which require authentication - here we just test a + # sample method to check the pattern works. mock_auth(MockMontaguAPI, requests_mock) - mock_successful_kill_response = { "status": "dead" } + mock_successful_kill_response = {"status": "dead"} requests_mock.post(f"{PACKIT_URL}/api/runner/cancel/test-task-id", [ {"status_code": 401, "text": json.dumps({"error": "Unauthorized"})}, {"status_code": 200, "text": json.dumps(mock_successful_kill_response)} @@ -146,22 +194,28 @@ def test_reauthenticates_on_401(MockMontaguAPI, requests_mock): assert resp == mock_successful_kill_response - assert_expected_packit_api_request(requests_mock, 1, "POST", f"{PACKIT_URL}/api/runner/cancel/test-task-id") + assert_expected_packit_api_request( + requests_mock, 1, "POST", + f"{PACKIT_URL}/api/runner/cancel/test-task-id") assert_expected_packit_auth_request(requests_mock, 2) - assert_expected_packit_api_request(requests_mock, 3, "POST", f"{PACKIT_URL}/api/runner/cancel/test-task-id") + assert_expected_packit_api_request( + requests_mock, 3, "POST", + f"{PACKIT_URL}/api/runner/cancel/test-task-id") + @patch("montagu.MontaguAPI") -def test_raises_exception_on_unexpected_status(MockMontaguAPI, requests_mock): +def test_raises_exception_on_unexpected_status( + MockMontaguAPI, + requests_mock): mock_auth(MockMontaguAPI, requests_mock) - # execute does not tolerate status codes other than 401 - should get an exception + # execute does not tolerate status codes other than 401 + # - should get an exception bad_response = {"error": "Bad request"} - requests_mock.get(f"{PACKIT_URL}/api/runner/status/test-task-id", status_code = 400, text = json.dumps(bad_response)) + requests_mock.get(f"{PACKIT_URL}/api/runner/status/test-task-id", + status_code=400, text=json.dumps(bad_response)) sut = PackitClient(config) with pytest.raises(PackitClientException) as exc_info: sut.poll("test-task-id") assert exc_info.value.response.status_code == 400 assert exc_info.value.response.json() == bad_response - - - diff --git a/test/unit/test_run_reports.py b/test/unit/test_run_reports.py index 7a634f4..341f68f 100644 --- a/test/unit/test_run_reports.py +++ b/test/unit/test_run_reports.py @@ -38,7 +38,8 @@ def test_run_reports(logging): "packetId": "r2-version"}] } - packit = MockPackitClient(expected_params, run_successfully, report_responses) + packit = MockPackitClient( + expected_params, run_successfully, report_responses) success_callback = Mock() error_callback = Mock() @@ -92,7 +93,8 @@ def test_run_reports_with_multi_hyphen_touchstone(): "r2": {"p1": "v1", "touchstone": "2021test-extra-1", "touchstone_name": "2021test-extra"} } - packit = MockPackitClient(expected_multi_params, run_successfully, report_responses) + packit = MockPackitClient(expected_multi_params, + run_successfully, report_responses) success_callback = Mock() error_callback = Mock() @@ -124,7 +126,8 @@ def test_run_reports_kills_currently_running(logging): "packetId": "r2-version"}] } - packit = MockPackitClient(expected_params, run_successfully, report_responses) + packit = MockPackitClient( + expected_params, run_successfully, report_responses) success_callback = Mock() error_callback = Mock() @@ -177,7 +180,8 @@ def test_run_reports_finish_on_different_poll_cycles(logging): ], "r2-key": [{"status": "COMPLETE", "packetId": "r2-version"}] } - packit = MockPackitClient(expected_params, run_successfully, report_responses) + packit = MockPackitClient(expected_params, run_successfully, + report_responses) success_callback = Mock() error_callback = Mock() @@ -207,8 +211,8 @@ def test_run_reports_finish_on_different_poll_cycles(logging): packit.kill_task.assert_not_called() success_callback.assert_has_calls([ - call(reports[1], "r2-version"), - call(reports[0], "r1-version") + call(reports[1], "r2-version"), + call(reports[0], "r1-version") ]) error_callback.assert_not_called() @@ -220,7 +224,8 @@ def test_run_reports_with_run_error(logging): "r2-key": [{"status": "COMPLETE", "packetId": "r2-version"}] } - packit = MockPackitClient(expected_params, run_successfully, report_responses) + packit = MockPackitClient(expected_params, run_successfully, + report_responses) success_callback = Mock() error_callback = Mock() @@ -272,7 +277,8 @@ def test_run_reports_with_status_error(logging): "packetId": "r2-version"}] } - packit = MockPackitClient(expected_params, run_successfully, report_responses) + packit = MockPackitClient(expected_params, run_successfully, + report_responses) success_callback = Mock() error_callback = Mock() @@ -313,7 +319,8 @@ def test_run_reports_with_status_failure(logging): "packetId": None}] } - packit = MockPackitClient(expected_params, run_successfully, report_responses) + packit = MockPackitClient(expected_params, run_successfully, + report_responses) success_callback = Mock() error_callback = Mock() @@ -341,7 +348,8 @@ def test_run_reports_with_status_failure(logging): mock_running_reports.assert_expected_calls() packit.kill_task.assert_not_called() success_callback.assert_called_with(reports[0], "r1-version") - error_callback.assert_called_with(reports[1], "Failure for key r2-key. Status: ERROR") + error_callback.assert_called_with(reports[1], + "Failure for key r2-key. Status: ERROR") @patch("src.utils.run_reports.logging") @@ -354,7 +362,8 @@ def test_run_reports_with_run_cancelled(logging): "packetId": None}] } - packit = MockPackitClient(expected_params, run_successfully, report_responses) + packit = MockPackitClient(expected_params, run_successfully, + report_responses) success_callback = Mock() error_callback = Mock() @@ -391,12 +400,13 @@ def test_run_reports_with_publish_failure(logging): run_successfully = ["r1", "r2"] report_responses = { "r1-key": [{"status": "COMPLETE", - "packetId": "r1-version"}], + "packetId": "r1-version"}], "r2-key": [{"status": "COMPLETE", - "packetId": "r2-version"}] + "packetId": "r2-version"}] } fail_publish = ["r2"] - packit = MockPackitClient(expected_params, run_successfully, report_responses, fail_publish) + packit = MockPackitClient(expected_params, run_successfully, + report_responses, fail_publish) success_callback = Mock() error_callback = Mock() @@ -453,8 +463,10 @@ def assert_expected_calls(self): call(group, disease, "r2", "r2-key") ], any_order=False) + class MockPackitClient: - def __init__(self, expected_params, run_successfully, report_responses, fail_publish = []): + def __init__(self, expected_params, run_successfully, report_responses, + fail_publish=[]): self.auth_success = True self.expected_params = expected_params self.run_successfully = run_successfully @@ -486,6 +498,7 @@ def publish(self, name, packit_id, roles): PACKIT_URL = "http://test-packit" + class MockConfig: def __init__(self, use_additional_recipients=True): From 8b96c897570d0e1b237942001624983a3ac7eb3a Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 16 Jul 2025 23:41:53 +0100 Subject: [PATCH 23/36] tweak db startup time --- scripts/run-dependencies.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/run-dependencies.sh b/scripts/run-dependencies.sh index e13d7e4..0b5c724 100755 --- a/scripts/run-dependencies.sh +++ b/scripts/run-dependencies.sh @@ -56,7 +56,7 @@ docker run -d \ $MONTAGU_PROXY_TAG 443 localhost # give packit api some time to migrate the db... -sleep 5 +sleep 10 # create roles to publish to... docker exec -i montagu-packit-db psql -U packituser -d packit --single-transaction < Date: Wed, 16 Jul 2025 23:46:03 +0100 Subject: [PATCH 24/36] try that again --- scripts/run-dependencies.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/run-dependencies.sh b/scripts/run-dependencies.sh index 0b5c724..1cc3d22 100755 --- a/scripts/run-dependencies.sh +++ b/scripts/run-dependencies.sh @@ -56,7 +56,8 @@ docker run -d \ $MONTAGU_PROXY_TAG 443 localhost # give packit api some time to migrate the db... -sleep 10 +# TODO: poll rather than ridiculous sleep +sleep 60 # create roles to publish to... docker exec -i montagu-packit-db psql -U packituser -d packit --single-transaction < Date: Wed, 16 Jul 2025 23:50:51 +0100 Subject: [PATCH 25/36] add logging --- scripts/run-dependencies.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/run-dependencies.sh b/scripts/run-dependencies.sh index 1cc3d22..b66695d 100755 --- a/scripts/run-dependencies.sh +++ b/scripts/run-dependencies.sh @@ -58,6 +58,8 @@ docker run -d \ # give packit api some time to migrate the db... # TODO: poll rather than ridiculous sleep sleep 60 +docker logs montagu-packit-db +docker logs montagu-packit-api # create roles to publish to... docker exec -i montagu-packit-db psql -U packituser -d packit --single-transaction < Date: Thu, 17 Jul 2025 10:29:47 +0100 Subject: [PATCH 26/36] update python version --- .github/workflows/test-and-push.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-and-push.yml b/.github/workflows/test-and-push.yml index 3c4a410..043182d 100644 --- a/.github/workflows/test-and-push.yml +++ b/.github/workflows/test-and-push.yml @@ -9,9 +9,9 @@ jobs: - uses: actions/checkout@v2 - name: Set up Python - uses: actions/setup-python@v2 + uses: actions/setup-python@v5 with: - python-version: 3.8 + python-version: 3.12 - name: Install dependencies run: | From d15f84ef0e481afffed342b851ffab1d8bb3dc6a Mon Sep 17 00:00:00 2001 From: Emma Date: Thu, 17 Jul 2025 10:41:28 +0100 Subject: [PATCH 27/36] separate out run dev deps scripts --- README.md | 2 +- scripts/run-dependencies.sh | 23 ++--------------------- scripts/run-dev-dependencies.sh | 22 ++++++++++++++++++++++ 3 files changed, 25 insertions(+), 22 deletions(-) create mode 100755 scripts/run-dev-dependencies.sh diff --git a/README.md b/README.md index 0f6f2a0..c664c2f 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ pip3 install -r requirements.txt ## Development -Run dependencies (Montagu API and DB, Proxy, Packit and a local Redis message queue in docker) with `scripts/run-dependencies.sh` +Run dependencies (Montagu API and DB, Proxy, Packit and a local Redis message queue in docker) with `scripts/run-dev-dependencies.sh` Dependencies also include a fake smtp server run from a [docker image](https://hub.docker.com/r/reachfive/fake-smtp-server) to enable development and testing of email functionality. You can see a web front end for the emails 'sent' via this server diff --git a/scripts/run-dependencies.sh b/scripts/run-dependencies.sh index b66695d..717eb98 100755 --- a/scripts/run-dependencies.sh +++ b/scripts/run-dependencies.sh @@ -56,10 +56,7 @@ docker run -d \ $MONTAGU_PROXY_TAG 443 localhost # give packit api some time to migrate the db... -# TODO: poll rather than ridiculous sleep -sleep 60 -docker logs montagu-packit-db -docker logs montagu-packit-api +sleep 5 # create roles to publish to... docker exec -i montagu-packit-db psql -U packituser -d packit --single-transaction < Date: Thu, 17 Jul 2025 10:49:05 +0100 Subject: [PATCH 28/36] test fix --- src/packit_client_exception.py | 2 +- src/utils/run_reports.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/packit_client_exception.py b/src/packit_client_exception.py index 280b917..1089ec5 100644 --- a/src/packit_client_exception.py +++ b/src/packit_client_exception.py @@ -2,7 +2,7 @@ class PackitClientException(Exception): def __init__(self, response): self.response = response json = response.json() - msg = "Unexpected response status from Packit API: " + + msg = "Unexpected response status from Packit API: " + \ f"{response.status_code}." if "error" in json and "detail" in json["error"]: msg = f"{msg} Detail: {json["error"]["detail"]}" diff --git a/src/utils/run_reports.py b/src/utils/run_reports.py index 70a37ca..a28cf79 100644 --- a/src/utils/run_reports.py +++ b/src/utils/run_reports.py @@ -112,12 +112,12 @@ def run_reports(packit, group, disease, touchstone, config, reports, report_config.publish_roles) if published: logging.info( - "Successfully published report packet " + + "Successfully published report packet " + \ f"{name} ({packet_id})" ) success_callback(report, packet_id) else: - error = "Failed to publish report packet " + + error = "Failed to publish report packet " + \ f"{name} ({packet_id})" logging.error(error) error_callback(report, error) From 66eac4a60f86f3d5225d5b964e10e3d6bbb04028 Mon Sep 17 00:00:00 2001 From: Emma Date: Thu, 17 Jul 2025 10:51:38 +0100 Subject: [PATCH 29/36] lint --- src/packit_client_exception.py | 2 +- src/utils/run_reports.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/packit_client_exception.py b/src/packit_client_exception.py index 1089ec5..725b935 100644 --- a/src/packit_client_exception.py +++ b/src/packit_client_exception.py @@ -3,7 +3,7 @@ def __init__(self, response): self.response = response json = response.json() msg = "Unexpected response status from Packit API: " + \ - f"{response.status_code}." + f"{response.status_code}." if "error" in json and "detail" in json["error"]: msg = f"{msg} Detail: {json["error"]["detail"]}" super().__init__(msg) diff --git a/src/utils/run_reports.py b/src/utils/run_reports.py index a28cf79..7c47170 100644 --- a/src/utils/run_reports.py +++ b/src/utils/run_reports.py @@ -112,13 +112,13 @@ def run_reports(packit, group, disease, touchstone, config, reports, report_config.publish_roles) if published: logging.info( - "Successfully published report packet " + \ - f"{name} ({packet_id})" + "Successfully published report packet" + + f" {name} ({packet_id})" ) success_callback(report, packet_id) else: - error = "Failed to publish report packet " + \ - f"{name} ({packet_id})" + error = "Failed to publish report packet" + \ + f" {name} ({packet_id})" logging.error(error) error_callback(report, error) new_packets[packet_id] = { From a0d2ff4cecea20123d438886cb59feb10608b0e6 Mon Sep 17 00:00:00 2001 From: Emma Date: Thu, 17 Jul 2025 17:49:58 +0100 Subject: [PATCH 30/36] more test fixes --- src/packit_client.py | 3 ++- src/packit_client_exception.py | 3 ++- test/integration/test_worker.py | 11 +++++------ test/unit/test_run_reports.py | 2 -- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/packit_client.py b/src/packit_client.py index 332e772..4252b53 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -112,7 +112,8 @@ def do_refresh_git(): def run(self, packet_group, parameters): def do_run(): - branch = "main" + # TODO: REVERT THIS!!! + branch = "mrc-6454-add-delay-to-diagnostic" #"main" commit = self.__get_latest_commit_for_branch(branch) data = { "name": packet_group, diff --git a/src/packit_client_exception.py b/src/packit_client_exception.py index 725b935..34336d8 100644 --- a/src/packit_client_exception.py +++ b/src/packit_client_exception.py @@ -5,5 +5,6 @@ def __init__(self, response): msg = "Unexpected response status from Packit API: " + \ f"{response.status_code}." if "error" in json and "detail" in json["error"]: - msg = f"{msg} Detail: {json["error"]["detail"]}" + detail = json["error"]["detail"] + msg = f"{msg} Detail: {detail}" super().__init__(msg) diff --git a/test/integration/test_worker.py b/test/integration/test_worker.py index d382d80..4811f11 100644 --- a/test/integration/test_worker.py +++ b/test/integration/test_worker.py @@ -6,7 +6,7 @@ from src.config import Config from src.utils.running_reports_repository import RunningReportsRepository -from src.orderlyweb_client_wrapper import OrderlyWebClientWrapper +from src.packit_client import PackitClient from test.integration.yt_utils import YouTrackUtils from test.integration.file_utils import write_text_file @@ -68,12 +68,11 @@ def test_later_task_kills_earlier_task_report(): assert len(versions) == 2 - # Check first report key's status with OrderlyWeb - should have been killed + # Check first report key's status with Packit - should have been killed config = Config() - wrapper = OrderlyWebClientWrapper(config) - result = wrapper.execute(wrapper.ow.report_status, first_report_key) - assert result.status == "interrupted" - assert result.finished + packit = PackitClient(config) + result = packit.poll(first_report_key) + assert result["status"] == "CANCELLED" # Check redis key has been tidied up assert running_repo.get("testGroup", "testDisease", "diagnostic") is None diff --git a/test/unit/test_run_reports.py b/test/unit/test_run_reports.py index 341f68f..8a07cda 100644 --- a/test/unit/test_run_reports.py +++ b/test/unit/test_run_reports.py @@ -1,8 +1,6 @@ from src.utils.run_reports import run_reports from src.config import ReportConfig -from orderlyweb_api import ReportStatusResult from unittest.mock import patch, call, Mock -from src.orderlyweb_client_wrapper import OrderlyWebClientWrapper reports = [ReportConfig("r1", None, ["r1@example.com"], "Subj: r1", "a.ssignee", ["Funders"]), From b68e5f70647b99ae1adf8c747428ac8e0c4f586f Mon Sep 17 00:00:00 2001 From: Emma Date: Sun, 20 Jul 2025 20:03:41 +0100 Subject: [PATCH 31/36] fix packit url in docker config --- config/docker_config.yml | 2 +- src/packit_client.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/config/docker_config.yml b/config/docker_config.yml index 85d9693..7d675f2 100644 --- a/config/docker_config.yml +++ b/config/docker_config.yml @@ -5,7 +5,7 @@ servers: user: test.user@example.com password: password packit: - url: http://montagu-packit-api:8080 + url: https://reverse-proxy:443/packit disable_certificate_verify: true youtrack: token: diff --git a/src/packit_client.py b/src/packit_client.py index 4252b53..a08f247 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -52,9 +52,11 @@ def __authenticate(self): monty = montagu.MontaguAPI(self.__config.montagu_url, self.__config.montagu_user, self.__config.montagu_password) + logging.info(f"MONTAGU TOKEN IS {monty.token}") packit_login_response = self.__get( "/auth/login/montagu", {"Authorization": f"Bearer {monty.token}"}) + logging.info(f"AUTH OK!!") self.auth_success = True self.token = packit_login_response["token"] self.__default_headers = { @@ -65,7 +67,7 @@ def __authenticate(self): self.auth_success = False logging.exception(ex) - def __execute(self, func, *args): # TODO: maybe don't need args? + def __execute(self, func, *args): # retry an operation if it fails auth (probably because of an expired # packit token) try: @@ -112,8 +114,7 @@ def do_refresh_git(): def run(self, packet_group, parameters): def do_run(): - # TODO: REVERT THIS!!! - branch = "mrc-6454-add-delay-to-diagnostic" #"main" + branch = "main" commit = self.__get_latest_commit_for_branch(branch) data = { "name": packet_group, From 9e74440012819b0b87be210a8715c074c711da3e Mon Sep 17 00:00:00 2001 From: Emma Date: Sun, 20 Jul 2025 20:56:10 +0100 Subject: [PATCH 32/36] lint --- src/packit_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/packit_client.py b/src/packit_client.py index a08f247..9bbec7e 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -114,7 +114,7 @@ def do_refresh_git(): def run(self, packet_group, parameters): def do_run(): - branch = "main" + branch = "main" commit = self.__get_latest_commit_for_branch(branch) data = { "name": packet_group, From 50a5d1cb63307195593f1cd447ae003ec5cd4b50 Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 21 Jul 2025 11:39:21 +0100 Subject: [PATCH 33/36] remove all orderlyweb references --- requirements.txt | 1 - scripts/orderly-web.yml | 57 -------------------------------- scripts/orderlyweb_cli.sh | 5 --- src/orderlyweb_client_wrapper.py | 33 ------------------ 4 files changed, 96 deletions(-) delete mode 100644 scripts/orderly-web.yml delete mode 100755 scripts/orderlyweb_cli.sh delete mode 100644 src/orderlyweb_client_wrapper.py diff --git a/requirements.txt b/requirements.txt index e0ba857..ccb6a43 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,6 @@ celery[redis] future pyyaml montagu>=0.0.2 -orderlyweb-api>=1.0.0 git+https://github.com/reside-ic/youtrack-rest-python-library constellation requests-mock diff --git a/scripts/orderly-web.yml b/scripts/orderly-web.yml deleted file mode 100644 index 13a51bf..0000000 --- a/scripts/orderly-web.yml +++ /dev/null @@ -1,57 +0,0 @@ -container_prefix: montagu_orderly - -network: montagu_default - -volumes: - orderly: orderly_volume - proxy_logs: orderly_web_proxy_logs - redis: orderly_web_redis_data -## Redis configuration -redis: - image: - name: redis - tag: "5.0" - volume: orderly_web_redis_data -## Orderly configuration -orderly: - image: - repo: vimc - name: orderly.server - tag: master - worker_name: orderly.server - initial: - source: clone - url: https://github.com/vimc/montagu-task-queue-orderly - -web: - image: - repo: vimc - name: orderly-web - tag: master - migrate: orderlyweb-migrate - admin: orderly-web-user-cli - url: https://localhost - dev_mode: true - port: 8888 - name: OrderlyWeb - email: admin@example.com - auth: - github_org: vimc - github_team: "" - github_oauth: - id: "notarealid" - secret: "notarealsecret" - fine_grained: true - montagu: true - montagu_url: http://montagu_api_1:8080 - montagu_api_url: http://montagu_api_1:8080/v1 - -proxy: - enabled: false - hostname: localhost - port_http: 80 - port_https: 443 - image: - repo: vimc - name: orderly-web-proxy - tag: master diff --git a/scripts/orderlyweb_cli.sh b/scripts/orderlyweb_cli.sh deleted file mode 100755 index 4117c92..0000000 --- a/scripts/orderlyweb_cli.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env bash - -image=${REGISTRY}/orderly-web-user-cli:master -docker pull $image -docker run --rm -v orderly_volume:/orderly --network ${NETWORK} $image $@ \ No newline at end of file diff --git a/src/orderlyweb_client_wrapper.py b/src/orderlyweb_client_wrapper.py deleted file mode 100644 index 0681af0..0000000 --- a/src/orderlyweb_client_wrapper.py +++ /dev/null @@ -1,33 +0,0 @@ -import montagu -import orderlyweb_api -import logging - - -def get_authorised_client(config): - try: - monty = montagu.MontaguAPI(config.montagu_url, config.montagu_user, - config.montagu_password) - ow = orderlyweb_api.OrderlyWebAPI(config.orderlyweb_url, - monty.token) - return ow - except Exception as ex: - logging.exception(ex) - return None - - -class OrderlyWebClientWrapper: - def __init__(self, config, auth=get_authorised_client): - self.config = config - self.auth = auth - self.ow = auth(config) - - def execute(self, func, *args): - try: - return func(*args) - except Exception as ex: - if "expired" in str(ex): - self.ow = self.auth(self.config) - func = getattr(self.ow, func.__name__) - return func(*args) - else: - raise ex From bb6b22d933db191fa95956aaeb640739a30b0032 Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 22 Jul 2025 09:35:16 +0100 Subject: [PATCH 34/36] remove extra logggin --- src/packit_client.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/packit_client.py b/src/packit_client.py index 9bbec7e..5202880 100644 --- a/src/packit_client.py +++ b/src/packit_client.py @@ -52,11 +52,9 @@ def __authenticate(self): monty = montagu.MontaguAPI(self.__config.montagu_url, self.__config.montagu_user, self.__config.montagu_password) - logging.info(f"MONTAGU TOKEN IS {monty.token}") packit_login_response = self.__get( "/auth/login/montagu", {"Authorization": f"Bearer {monty.token}"}) - logging.info(f"AUTH OK!!") self.auth_success = True self.token = packit_login_response["token"] self.__default_headers = { @@ -140,7 +138,7 @@ def do_kill_task(): def publish(self, name, packet_id, roles): # mimic OW publishing by setting packet-level permission for a new # report packet permission on a list of configured roles. - # NB: These role can either be user # roles or groups. + # NB: These role can either be user roles or groups. # If users, these need to be user names not email addresses. def do_publish_to_role(role): data = { From dfb4f0138f1d25ebdb56010442a3af3cf0613d28 Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 23 Jul 2025 16:59:28 +0100 Subject: [PATCH 35/36] get all tags --- src/task_run_diagnostic_reports.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/task_run_diagnostic_reports.py b/src/task_run_diagnostic_reports.py index a8b7346..ed4ca86 100644 --- a/src/task_run_diagnostic_reports.py +++ b/src/task_run_diagnostic_reports.py @@ -117,14 +117,15 @@ def create_ticket(group, disease, touchstone, scenario, def create_tag(yt, tag_name): + logging.info(f"Creating YouTrack tag: {tag_name}") try: - yt.create_tag(tag_name) - except YTException: - logging.error(f"Failed to create YouTrack tag {tag_name}") + yt.create_tag(tag_name, return_fields=["name"]) + except YTException as ex: + logging.error(f"Failed to create YouTrack tag {tag_name}: {ex}") def create_tags(yt, group, disease, touchstone, report): - tags = yt.get_tags(fields=["name"]) + tags = yt.get_tags(fields=["name"], top=10000) if len([t for t in tags if t["name"] == disease]) == 0: create_tag(yt, disease) if len([t for t in tags if t["name"] == group]) == 0: From 8f3d24dcfbde1df1bc2d79c5212b5cee42f046ef Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 23 Jul 2025 18:23:36 +0100 Subject: [PATCH 36/36] remove unnecessary fields param --- src/task_run_diagnostic_reports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/task_run_diagnostic_reports.py b/src/task_run_diagnostic_reports.py index ed4ca86..9d07352 100644 --- a/src/task_run_diagnostic_reports.py +++ b/src/task_run_diagnostic_reports.py @@ -119,7 +119,7 @@ def create_ticket(group, disease, touchstone, scenario, def create_tag(yt, tag_name): logging.info(f"Creating YouTrack tag: {tag_name}") try: - yt.create_tag(tag_name, return_fields=["name"]) + yt.create_tag(tag_name) except YTException as ex: logging.error(f"Failed to create YouTrack tag {tag_name}: {ex}")