From 6af4ebed2bd121963042faca7629f1211c0418d3 Mon Sep 17 00:00:00 2001 From: Oleg Kalashev Date: Mon, 6 Nov 2023 16:20:00 +0300 Subject: [PATCH 1/9] ProgressReporter class added --- oda_api/api.py | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/oda_api/api.py b/oda_api/api.py index f5b5d6d6..1faa0d26 100644 --- a/oda_api/api.py +++ b/oda_api/api.py @@ -36,6 +36,7 @@ import requests import ast import json +import re try: # compatibility in some remaining environments @@ -1339,3 +1340,46 @@ def from_response_json(cls, res_json, instrument, product): p.meta_data = p.meta return d + +class ProgressReporter(object): + """ + The class allows to report task progress to end user + """ + def __init__(self): + self._callback = None + callback_file = ".oda_api_callback" # perhaps it would be better to define this constant in a common lib + if not os.path.isfile(callback_file): + return + with open(callback_file, 'r') as file: + self._callback = file.read().strip() + + @property + def enabled(self): + return self._callback is not None + + def report_progress(self, message: str): + """ + Report progress via callback URL + :param message: message to pass + """ + if not self.enabled: + logger.info('no callback registered, skipping') + return + + logger.info('will perform callback: %s', self._callback) + + callback_payload = dict( + message=message + ) + + if re.match('^file://', self._callback): + with open(self._callback.replace('file://', ''), "w") as f: + json.dump(callback_payload, f) + logger.info('stored callback in a file %s', self._callback) + + elif re.match('^https?://', self._callback): + r = requests.get(self._callback, params=callback_payload) + logger.info('callback %s returns %s : %s', self._callback, r, r.text) + + else: + raise NotImplementedError From bf1aa52572b9a6d48008ac5af47f72e85854f6b1 Mon Sep 17 00:00:00 2001 From: Oleg Kalashev Date: Tue, 7 Nov 2023 14:14:09 +0300 Subject: [PATCH 2/9] ProgressReporter.report_progress signature was changed --- oda_api/api.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/oda_api/api.py b/oda_api/api.py index 1faa0d26..5bafed7f 100644 --- a/oda_api/api.py +++ b/oda_api/api.py @@ -1357,21 +1357,23 @@ def __init__(self): def enabled(self): return self._callback is not None - def report_progress(self, message: str): + def report_progress(self, stage: str=None, progress: int=50, substage: str=None, subprogress: int=None, message:str=None): """ Report progress via callback URL + :param stage: current stage description string + :param stage: current stage progress in % + :param stage: current substage description string + :param stage: current substage progress in % :param message: message to pass """ + callback_payload = {k: str(v) for k, v in locals().items() if v is not None and k != 'self'} + if not self.enabled: logger.info('no callback registered, skipping') return logger.info('will perform callback: %s', self._callback) - callback_payload = dict( - message=message - ) - if re.match('^file://', self._callback): with open(self._callback.replace('file://', ''), "w") as f: json.dump(callback_payload, f) From d47ff6eac8048cc370a07ba337c2f5b5db341614 Mon Sep 17 00:00:00 2001 From: Oleg Kalashev Date: Fri, 17 Nov 2023 19:23:58 +0300 Subject: [PATCH 3/9] help string updated for report_progress --- oda_api/api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/oda_api/api.py b/oda_api/api.py index 5bafed7f..e7e4cf13 100644 --- a/oda_api/api.py +++ b/oda_api/api.py @@ -1361,9 +1361,9 @@ def report_progress(self, stage: str=None, progress: int=50, substage: str=None, """ Report progress via callback URL :param stage: current stage description string - :param stage: current stage progress in % - :param stage: current substage description string - :param stage: current substage progress in % + :param progress: current stage progress in % + :param substage: current substage description string + :param subprogress: current substage progress in % :param message: message to pass """ callback_payload = {k: str(v) for k, v in locals().items() if v is not None and k != 'self'} From ce889d8812660bbe36effc453a905ca1c1a4b22e Mon Sep 17 00:00:00 2001 From: Oleg Kalashev Date: Fri, 17 Nov 2023 19:36:47 +0300 Subject: [PATCH 4/9] test_progress_report was added --- tests/test_progress_report.py | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 tests/test_progress_report.py diff --git a/tests/test_progress_report.py b/tests/test_progress_report.py new file mode 100644 index 00000000..6bdffe9b --- /dev/null +++ b/tests/test_progress_report.py @@ -0,0 +1,6 @@ +from oda_api.api import ProgressReporter + +def test_progress_reporter(): + pr = ProgressReporter() + assert not pr.enabled + pr.report_progress(stage='simulation', progress=50, substage='spectra', subprogress=30, message='some message') From 0c74c9208cc76ab56e1d0f3c88c6f1df9feea413 Mon Sep 17 00:00:00 2001 From: Oleg Kalashev Date: Fri, 17 Nov 2023 22:42:06 +0300 Subject: [PATCH 5/9] report_progress refactored, extra test added --- oda_api/api.py | 3 ++- tests/test_progress_report.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/oda_api/api.py b/oda_api/api.py index e7e4cf13..3838b28d 100644 --- a/oda_api/api.py +++ b/oda_api/api.py @@ -1366,7 +1366,8 @@ def report_progress(self, stage: str=None, progress: int=50, substage: str=None, :param subprogress: current substage progress in % :param message: message to pass """ - callback_payload = {k: str(v) for k, v in locals().items() if v is not None and k != 'self'} + callback_payload = dict(stage=stage, progress=progress, substage=substage, subprogress=subprogress, message=message) + callback_payload = {k: v for k, v in callback_payload.items() if v is not None} if not self.enabled: logger.info('no callback registered, skipping') diff --git a/tests/test_progress_report.py b/tests/test_progress_report.py index 6bdffe9b..e9f888b3 100644 --- a/tests/test_progress_report.py +++ b/tests/test_progress_report.py @@ -1,6 +1,34 @@ from oda_api.api import ProgressReporter +import os +import json -def test_progress_reporter(): +callback_file = ".oda_api_callback" + +def test_progress_reporter_disabled(): + if os.path.isfile(callback_file): + os.remove(callback_file) + # if callback is not available pr = ProgressReporter() assert not pr.enabled pr.report_progress(stage='simulation', progress=50, substage='spectra', subprogress=30, message='some message') + +def test_progress_reporter_enabled(): + # if callback is available + try: + dump_file = 'callback' + with open(callback_file, 'w') as file: + print(f'file://{os.getcwd()}/{dump_file}', file=file) + + pr = ProgressReporter() + assert pr.enabled + params = dict(stage='simulation', progress=50, substage='spectra', subprogress=30, message='some message') + pr.report_progress(**params) + with open(dump_file) as json_file: + passed_params = json.load(json_file) + assert len([k for k in passed_params.keys() if k not in params]) == 0 + finally: + if os.path.isfile(callback_file): + os.remove(callback_file) + if os.path.isfile(dump_file): + os.remove(dump_file) + From 6c25ca17958520105cb81c68f56096ce3764c167 Mon Sep 17 00:00:00 2001 From: Oleg Kalashev Date: Wed, 6 Dec 2023 18:19:29 +0100 Subject: [PATCH 6/9] Update oda_api/api.py Co-authored-by: Denys Savchenko <56398430+dsavchenko@users.noreply.github.com> --- oda_api/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oda_api/api.py b/oda_api/api.py index 3838b28d..4dc2dc2c 100644 --- a/oda_api/api.py +++ b/oda_api/api.py @@ -1368,7 +1368,7 @@ def report_progress(self, stage: str=None, progress: int=50, substage: str=None, """ callback_payload = dict(stage=stage, progress=progress, substage=substage, subprogress=subprogress, message=message) callback_payload = {k: v for k, v in callback_payload.items() if v is not None} - + callback_payload['action'] = 'progress' if not self.enabled: logger.info('no callback registered, skipping') return From 1c71c7e2fe705208fa9b21645384da4a0d3800c7 Mon Sep 17 00:00:00 2001 From: Oleg Kalashev Date: Wed, 6 Dec 2023 18:46:32 +0100 Subject: [PATCH 7/9] tests updated to address reviewer's comments --- tests/test_progress_report.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/test_progress_report.py b/tests/test_progress_report.py index e9f888b3..a1e38d55 100644 --- a/tests/test_progress_report.py +++ b/tests/test_progress_report.py @@ -3,6 +3,7 @@ import json callback_file = ".oda_api_callback" +request_params = dict(stage='simulation', progress=50, substage='spectra', subprogress=30, message='some message') def test_progress_reporter_disabled(): if os.path.isfile(callback_file): @@ -10,7 +11,11 @@ def test_progress_reporter_disabled(): # if callback is not available pr = ProgressReporter() assert not pr.enabled - pr.report_progress(stage='simulation', progress=50, substage='spectra', subprogress=30, message='some message') + # the call below should not produce exception + try: + pr.report_progress(**request_params) + except: + assert False, 'report_progress raises exception in case of disabled ProgressReporter' def test_progress_reporter_enabled(): # if callback is available @@ -21,11 +26,13 @@ def test_progress_reporter_enabled(): pr = ProgressReporter() assert pr.enabled - params = dict(stage='simulation', progress=50, substage='spectra', subprogress=30, message='some message') - pr.report_progress(**params) + + pr.report_progress(**request_params) + + # verify that params passed to report_progress were saved to dump_file with open(dump_file) as json_file: - passed_params = json.load(json_file) - assert len([k for k in passed_params.keys() if k not in params]) == 0 + saved_params = json.load(json_file) + assert saved_params == request_params finally: if os.path.isfile(callback_file): os.remove(callback_file) From 5044ac40441ec17fea061dbc8a1a20a68c6d66d3 Mon Sep 17 00:00:00 2001 From: Oleg Kalashev Date: Wed, 6 Dec 2023 22:23:41 +0100 Subject: [PATCH 8/9] ProgressReporter documentation was added --- doc/source/user_guide/ProgressReporter.ipynb | 97 ++++++++++++++++++++ doc/source/user_guide/tutorial_main.rst | 2 + 2 files changed, 99 insertions(+) create mode 100644 doc/source/user_guide/ProgressReporter.ipynb diff --git a/doc/source/user_guide/ProgressReporter.ipynb b/doc/source/user_guide/ProgressReporter.ipynb new file mode 100644 index 00000000..f399459f --- /dev/null +++ b/doc/source/user_guide/ProgressReporter.ipynb @@ -0,0 +1,97 @@ +{ + "cells": [ + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "# Progress reporting for long running tasks" + ] + }, + { + "cell_type": "code", + "execution_count": 1, + "metadata": {}, + "outputs": [], + "source": [ + "from oda_api.api import ProgressReporter\n", + "import time" + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "## Initialise ProgressReporter" + ] + }, + { + "cell_type": "code", + "execution_count": 2, + "metadata": {}, + "outputs": [], + "source": [ + "pr = ProgressReporter()" + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "## Split your task into subtasks to enable progress report\n", + "use ProgressReporter.report_progress method to send progress reports" + ] + }, + { + "cell_type": "code", + "execution_count": 3, + "metadata": {}, + "outputs": [], + "source": [ + "n_steps = 5\n", + "for step in range(n_steps):\n", + " stage = f\"simulation stage {step}\"\n", + " progress = 100 * step // n_steps\n", + " n_substeps = 10\n", + " \n", + " # optionally define subtasks\n", + " for substep in range(n_substeps):\n", + " substage = f\"subtask {substep}\"\n", + " subtask_progress = 100 * substep // n_substeps\n", + "\n", + " time.sleep(0.001) # replace this by actual calculation\n", + "\n", + " # report progress, optionally adding extra message\n", + " message='some message'\n", + " \n", + " pr.report_progress(stage=stage, progress=progress, substage=substage, subprogress=subtask_progress, message=message)" + ] + } + ], + "metadata": { + "kernel_info": { + "name": "python2" + }, + "kernelspec": { + "display_name": "Python 3 (ipykernel)", + "language": "python", + "name": "python3" + }, + "language_info": { + "codemirror_mode": { + "name": "ipython", + "version": 3 + }, + "file_extension": ".py", + "mimetype": "text/x-python", + "name": "python", + "nbconvert_exporter": "python", + "pygments_lexer": "ipython3", + "version": "3.9.18" + }, + "nteract": { + "version": "0.15.0" + } + }, + "nbformat": 4, + "nbformat_minor": 4 +} diff --git a/doc/source/user_guide/tutorial_main.rst b/doc/source/user_guide/tutorial_main.rst index 514b5dbd..841de97f 100644 --- a/doc/source/user_guide/tutorial_main.rst +++ b/doc/source/user_guide/tutorial_main.rst @@ -25,6 +25,8 @@ The following tutorial covers a large fraction of the code features. The example Upload a product to the Product Gallery + Progress reporting for long running tasks + Examples of workflows From a13948db727d1b936ea6f1c3b40b6658d01000a6 Mon Sep 17 00:00:00 2001 From: Oleg Kalashev Date: Mon, 18 Dec 2023 14:57:32 +0300 Subject: [PATCH 9/9] bug fixing tests/test_progress_report.py --- tests/test_progress_report.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_progress_report.py b/tests/test_progress_report.py index a1e38d55..4c3079dd 100644 --- a/tests/test_progress_report.py +++ b/tests/test_progress_report.py @@ -32,6 +32,8 @@ def test_progress_reporter_enabled(): # verify that params passed to report_progress were saved to dump_file with open(dump_file) as json_file: saved_params = json.load(json_file) + # append extra param befor check + request_params['action'] = 'progress' # this key is added by report_progress assert saved_params == request_params finally: if os.path.isfile(callback_file):