From bdfb141e8e80fc974cc32fe0c20aa92e2753a27a Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Tue, 16 Jul 2024 15:05:19 +0200 Subject: [PATCH 01/14] Modified default location of reports. Modified MANIFEST to include static folder --- cornflow-server/MANIFEST.in | 3 ++- cornflow-server/cornflow/config.py | 2 +- cornflow-server/static/__init__.py | 0 3 files changed, 3 insertions(+), 2 deletions(-) delete mode 100644 cornflow-server/static/__init__.py diff --git a/cornflow-server/MANIFEST.in b/cornflow-server/MANIFEST.in index 4f047318..3816e709 100644 --- a/cornflow-server/MANIFEST.in +++ b/cornflow-server/MANIFEST.in @@ -3,4 +3,5 @@ include MANIFEST.in include README.rst include setup.py include cornflow/migrations/* -include cornflow/migrations/versions/* \ No newline at end of file +include cornflow/migrations/versions/* +include cornflow/static/* \ No newline at end of file diff --git a/cornflow-server/cornflow/config.py b/cornflow-server/cornflow/config.py index 4dba2613..117d3477 100644 --- a/cornflow-server/cornflow/config.py +++ b/cornflow-server/cornflow/config.py @@ -26,7 +26,7 @@ class DefaultConfig(object): FILE_BACKEND = os.getenv("FILE_BACKEND", "local") UPLOAD_FOLDER = os.getenv( "UPLOAD_FOLDER", - os.path.abspath(os.path.join(os.path.dirname(__file__), "../static")), + os.path.abspath(os.path.join(os.path.dirname(__file__), "./static")), ) ALLOWED_EXTENSIONS = os.getenv("ALLOWED_EXTENSIONS", ["pdf", "html"]) diff --git a/cornflow-server/static/__init__.py b/cornflow-server/static/__init__.py deleted file mode 100644 index e69de29b..00000000 From b2ca5c1727c73c5eb0e8e446e9458c30c5eb7053 Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Tue, 16 Jul 2024 16:01:14 +0200 Subject: [PATCH 02/14] More fixes to tests --- cornflow-server/cornflow/config.py | 2 + cornflow-server/cornflow/endpoints/reports.py | 2 +- .../cornflow_client/raw_cornflow_client.py | 18 ++--- libs/client/cornflow_client/tests/const.py | 1 + .../integration/test_cornflow_integration.py | 13 +++- .../test_raw_cornflow_integration.py | 76 ++++++++++++++++--- 6 files changed, 87 insertions(+), 25 deletions(-) diff --git a/cornflow-server/cornflow/config.py b/cornflow-server/cornflow/config.py index 117d3477..a33e7d73 100644 --- a/cornflow-server/cornflow/config.py +++ b/cornflow-server/cornflow/config.py @@ -95,6 +95,7 @@ class Development(DefaultConfig): """ """ ENV = "development" + UPLOAD_FOLDER = os.getenv("UPLOAD_FOLDER", "/usr/src/app/static") class Testing(DefaultConfig): @@ -126,6 +127,7 @@ class Production(DefaultConfig): # needs to be on to avoid getting only 500 codes: # and https://medium.com/@johanesriandy/flask-error-handler-not-working-on-production-mode-3adca4c7385c PROPAGATE_EXCEPTIONS = True + UPLOAD_FOLDER = os.getenv("UPLOAD_FOLDER", "/usr/src/app/static") app_config = {"development": Development, "testing": Testing, "production": Production} diff --git a/cornflow-server/cornflow/endpoints/reports.py b/cornflow-server/cornflow/endpoints/reports.py index e6c034cc..0e1e073f 100644 --- a/cornflow-server/cornflow/endpoints/reports.py +++ b/cornflow-server/cornflow/endpoints/reports.py @@ -70,7 +70,7 @@ def post(self, **kwargs): the reference_id for the newly created report if successful) and a integer with the HTTP status code :rtype: Tuple(dict, integer) """ - execution = ExecutionModel.get_one_object(id=kwargs["execution_id"]) + execution = ExecutionModel.get_one_object(idx=kwargs["execution_id"]) if execution is None: raise ObjectDoesNotExist("The execution does not exist") diff --git a/libs/client/cornflow_client/raw_cornflow_client.py b/libs/client/cornflow_client/raw_cornflow_client.py index 4946e4e7..2d4be87f 100644 --- a/libs/client/cornflow_client/raw_cornflow_client.py +++ b/libs/client/cornflow_client/raw_cornflow_client.py @@ -530,24 +530,22 @@ def create_report(self, name, filename, execution_id, encoding=None, **kwargs): :param str encoding: the type of encoding used in the call. Defaults to 'br' """ with open(filename, "rb") as _file: - payload = ( - dict(file=_file, name=name, execution_id=execution_id, **kwargs), + result = self.create_api( + "report/", + data=dict(name=name, execution_id=execution_id, **kwargs), + files=dict(file=_file), + encoding=encoding, + headers={"content_type": "multipart/form-data"}, ) - result = self.create_api( - "report/", - data=payload, - encoding=encoding, - headers={"content_type": "multipart/form-data"}, - ) return result @ask_token @prepare_encoding def get_one_report(self, reference_id, folder_destination, encoding=None): result = self.get_api_for_id(api="report", id=reference_id, encoding=encoding) - content = result.content() + content = result.content - file_name = result.headers["Content-Disposition"].split["="][1] + file_name = result.headers["Content-Disposition"].split("=")[1] path = os.path.normpath(os.path.join(folder_destination, file_name)) diff --git a/libs/client/cornflow_client/tests/const.py b/libs/client/cornflow_client/tests/const.py index ba21f4a1..930e75fe 100644 --- a/libs/client/cornflow_client/tests/const.py +++ b/libs/client/cornflow_client/tests/const.py @@ -196,6 +196,7 @@ def _get_file(relative_path): PULP_EXAMPLE = _get_file("./data/pulp_example_data.json") HTML_REPORT = "../data/new_report.html" +TEST_FOLDER = "./" PUBLIC_DAGS = [ "solve_model_dag", diff --git a/libs/client/cornflow_client/tests/integration/test_cornflow_integration.py b/libs/client/cornflow_client/tests/integration/test_cornflow_integration.py index 38bf1be2..742d553b 100644 --- a/libs/client/cornflow_client/tests/integration/test_cornflow_integration.py +++ b/libs/client/cornflow_client/tests/integration/test_cornflow_integration.py @@ -11,7 +11,7 @@ import pulp as pl -from cornflow_client import CornFlow +from cornflow_client import CornFlow, CornFlowApiError from cornflow_client.constants import STATUS_OPTIMAL, STATUS_NOT_SOLVED, STATUS_QUEUED from cornflow_client.schema.tools import get_pulp_jsonschema from cornflow_client.tests.const import PUBLIC_DAGS, PULP_EXAMPLE @@ -551,9 +551,14 @@ def setUp(self): login_result = self.client.login("admin", "Adminpassword1!") self.assertIn("id", login_result.keys()) self.assertIn("token", login_result.keys()) - self.base_user_id = CornFlow(url="http://127.0.0.1:5050/").login( - "user", "UserPassword1!" - )["id"] + try: + self.base_user_id = CornFlow(url="http://127.0.0.1:5050/").login( + "user", "UserPassword1!" + )["id"] + except CornFlowApiError: + self.base_user_id = CornFlow(url="http://127.0.0.1:5050/").sign_up( + username="user", pwd="UserPassword1!", email="user@cornflow.org" + )["id"] def tearDown(self): pass diff --git a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py index 2c1e7626..2f1c168e 100644 --- a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py +++ b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py @@ -11,10 +11,15 @@ import pulp as pl -from cornflow_client import CornFlow +from cornflow_client import CornFlow, CornFlowApiError from cornflow_client.constants import STATUS_OPTIMAL, STATUS_NOT_SOLVED, STATUS_QUEUED from cornflow_client.schema.tools import get_pulp_jsonschema -from cornflow_client.tests.const import PUBLIC_DAGS, PULP_EXAMPLE, HTML_REPORT +from cornflow_client.tests.const import ( + PUBLIC_DAGS, + PULP_EXAMPLE, + HTML_REPORT, + TEST_FOLDER, +) # Constants path_to_tests_dir = os.path.dirname(os.path.abspath(__file__)) @@ -35,7 +40,12 @@ def _get_file(relative_path): class TestRawCornflowClientUser(TestCase): def setUp(self): self.client = CornFlow(url="http://127.0.0.1:5050/") - login_result = self.client.raw.login("user", "UserPassword1!") + try: + login_result = self.client.raw.login("user", "UserPassword1!") + except CornFlowApiError: + login_result = self.client.raw.sign_up( + username="user", pwd="UserPassword1!", email="user@cornflow.org" + ) data = login_result.json() self.assertEqual(login_result.status_code, 200) self.assertIn("id", data.keys()) @@ -46,9 +56,9 @@ def tearDown(self): pass def check_execution_statuses( - self, execution_id, end_state=STATUS_OPTIMAL, initial_state=None + self, execution_id, end_state=STATUS_OPTIMAL, initial_state=STATUS_QUEUED ): - if initial_state is None: + if initial_state is not None: statuses = [initial_state] else: statuses = [] @@ -332,7 +342,9 @@ def test_get_execution_log(self): def test_get_execution_solution(self): execution = self.test_create_execution() - statuses = self.check_execution_statuses(execution["id"]) + statuses = self.check_execution_statuses( + execution["id"], initial_state=STATUS_QUEUED + ) response = self.client.raw.get_solution(execution["id"]) self.assertEqual(response.status_code, 200) @@ -612,9 +624,14 @@ def setUp(self): login_result = self.client.login("admin", "Adminpassword1!") self.assertIn("id", login_result.keys()) self.assertIn("token", login_result.keys()) - self.base_user_id = CornFlow(url="http://127.0.0.1:5050/").login( - "user", "UserPassword1!" - )["id"] + try: + self.base_user_id = CornFlow(url="http://127.0.0.1:5050/").login( + "user", "UserPassword1!" + )["id"] + except CornFlowApiError: + self.base_user_id = CornFlow(url="http://127.0.0.1:5050/").sign_up( + username="user", pwd="UserPassword1!", email="user@cornflow.org" + )["id"] def tearDown(self): pass @@ -642,6 +659,14 @@ def setUp(self): login_result = self.client.login("airflow", "Airflow_test_password1") self.assertIn("id", login_result.keys()) self.assertIn("token", login_result.keys()) + try: + self.base_user_id = CornFlow(url="http://127.0.0.1:5050/").login( + "user", "UserPassword1!" + )["id"] + except CornFlowApiError: + self.base_user_id = CornFlow(url="http://127.0.0.1:5050/").sign_up( + username="user", pwd="UserPassword1!", email="user@cornflow.org" + )["id"] def tearDown(self): pass @@ -752,4 +777,35 @@ def test_post_report_html(self): run=False, ).json() - client.raw.create_report("new_report", HTML_REPORT, execution["id"]) + print(execution["id"]) + + response = self.client.raw.create_report( + "new_report", HTML_REPORT, execution["id"] + ) + + self.assertEqual(response.status_code, 201) + + return response + + def test_get_one_report(self): + response = self.test_post_report_html() + print(response.json()) + report_id = response.json()["id"] + + client = CornFlow(url="http://127.0.0.1:5050/") + _ = client.login("user", "UserPassword1!") + + response = client.get_one_report( + reference_id=report_id, folder_destination=TEST_FOLDER + ) + self.assertEqual(response.status_code, 200) + + # read from TEST FOLDER + with open(os.path.join(TEST_FOLDER, "new_report.html"), "r") as f: + file = f.read() + + # read from test/data folder + with open(HTML_REPORT, "r") as f: + file_2 = f.read() + + self.assertEqual(file, file_2) From 778aad271bd96478b3a28b6b21bd12a333403884 Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Tue, 16 Jul 2024 16:56:58 +0200 Subject: [PATCH 03/14] Modified cornflow dockerfile --- cornflow-server/Dockerfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cornflow-server/Dockerfile b/cornflow-server/Dockerfile index 851f6e30..bbf9bc50 100644 --- a/cornflow-server/Dockerfile +++ b/cornflow-server/Dockerfile @@ -36,6 +36,9 @@ RUN pip install "cornflow==${CORNFLOW_VERSION}" # create folder for logs RUN mkdir -p /usr/src/app/log +# create folder for object storage +RUN mkdir -p /usr/src/app/static + # create folder for custom ssh keys RUN mkdir /usr/src/app/.ssh From 68f49897c9d929eff833d8dbe1bdc973441afaca Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Tue, 16 Jul 2024 19:02:50 +0200 Subject: [PATCH 04/14] Modified default location of reports. Modified MANIFEST to include static folder --- .github/workflows/test_cornflow_server.yml | 2 +- libs/client/cornflow_client/tests/const.py | 2 +- .../tests/integration/test_raw_cornflow_integration.py | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test_cornflow_server.yml b/.github/workflows/test_cornflow_server.yml index 4e8fc457..6769fd2e 100644 --- a/.github/workflows/test_cornflow_server.yml +++ b/.github/workflows/test_cornflow_server.yml @@ -49,7 +49,7 @@ jobs: steps: - uses: actions/checkout@v1 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v1 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - name: Copy DAG files diff --git a/libs/client/cornflow_client/tests/const.py b/libs/client/cornflow_client/tests/const.py index 930e75fe..e6f39412 100644 --- a/libs/client/cornflow_client/tests/const.py +++ b/libs/client/cornflow_client/tests/const.py @@ -195,7 +195,7 @@ def _get_file(relative_path): ) PULP_EXAMPLE = _get_file("./data/pulp_example_data.json") -HTML_REPORT = "../data/new_report.html" +HTML_REPORT = _get_file("./data/new_report.html") TEST_FOLDER = "./" PUBLIC_DAGS = [ diff --git a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py index 2f1c168e..1e856953 100644 --- a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py +++ b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py @@ -795,7 +795,7 @@ def test_get_one_report(self): client = CornFlow(url="http://127.0.0.1:5050/") _ = client.login("user", "UserPassword1!") - response = client.get_one_report( + response = client.raw.get_one_report( reference_id=report_id, folder_destination=TEST_FOLDER ) self.assertEqual(response.status_code, 200) @@ -809,3 +809,6 @@ def test_get_one_report(self): file_2 = f.read() self.assertEqual(file, file_2) + + # remove file from TEST_FOLDER + os.remove(os.path.join(TEST_FOLDER, "new_report.html")) From 824d4729ce5766a3a37698d1cfb13e7f23380b05 Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Wed, 17 Jul 2024 09:17:39 +0200 Subject: [PATCH 05/14] Some print and error catching to get the error on Github actions --- cornflow-server/cornflow/endpoints/reports.py | 69 ++++++++++--------- .../test_raw_cornflow_integration.py | 6 +- 2 files changed, 41 insertions(+), 34 deletions(-) diff --git a/cornflow-server/cornflow/endpoints/reports.py b/cornflow-server/cornflow/endpoints/reports.py index 0e1e073f..85f2f956 100644 --- a/cornflow-server/cornflow/endpoints/reports.py +++ b/cornflow-server/cornflow/endpoints/reports.py @@ -22,6 +22,7 @@ FileError, ObjectDoesNotExist, NoPermission, + InvalidUsage, ) @@ -70,49 +71,53 @@ def post(self, **kwargs): the reference_id for the newly created report if successful) and a integer with the HTTP status code :rtype: Tuple(dict, integer) """ - execution = ExecutionModel.get_one_object(idx=kwargs["execution_id"]) + try: + execution = ExecutionModel.get_one_object(idx=kwargs["execution_id"]) - if execution is None: - raise ObjectDoesNotExist("The execution does not exist") + if execution is None: + raise ObjectDoesNotExist("The execution does not exist") - if "file" not in request.files: - return {"message": "No file part"}, 400 + if "file" not in request.files: + return {"message": "No file part"}, 400 - file = request.files["file"] - filename = secure_filename(file.filename) - filename_extension = filename.split(".")[-1] + file = request.files["file"] + filename = secure_filename(file.filename) + filename_extension = filename.split(".")[-1] - if filename_extension not in current_app.config["ALLOWED_EXTENSIONS"]: - return { - "message": f"Invalid file extension. " - f"Valid extensions are: {current_app.config['ALLOWED_EXTENSIONS']}" - }, 400 + if filename_extension not in current_app.config["ALLOWED_EXTENSIONS"]: + return { + "message": f"Invalid file extension. " + f"Valid extensions are: {current_app.config['ALLOWED_EXTENSIONS']}" + }, 400 - my_directory = f"{current_app.config['UPLOAD_FOLDER']}/{execution.id}" + my_directory = f"{current_app.config['UPLOAD_FOLDER']}/{execution.id}" - # we create a directory for the execution - if not os.path.exists(my_directory): - current_app.logger.info(f"Creating directory {my_directory}") - os.mkdir(my_directory) + # we create a directory for the execution + if not os.path.exists(my_directory): + current_app.logger.info(f"Creating directory {my_directory}") + os.mkdir(my_directory) - report_name = f"{secure_filename(kwargs['name'])}.{filename_extension}" + report_name = f"{secure_filename(kwargs['name'])}.{filename_extension}" - save_path = os.path.normpath(os.path.join(my_directory, report_name)) + save_path = os.path.normpath(os.path.join(my_directory, report_name)) - if "static" not in save_path or ".." in save_path: - raise NoPermission("Invalid file name") + if "static" not in save_path or ".." in save_path: + raise NoPermission("Invalid file name") - report = ReportModel( - { - "name": kwargs["name"], - "file_url": save_path, - "execution_id": kwargs["execution_id"], - "user_id": execution.user_id, - "description": kwargs.get("description", ""), - } - ) + report = ReportModel( + { + "name": kwargs["name"], + "file_url": save_path, + "execution_id": kwargs["execution_id"], + "user_id": execution.user_id, + "description": kwargs.get("description", ""), + } + ) - report.save() + report.save() + except Exception as error: + current_app.logger.error(error) + raise InvalidUsage("Error on POST report") try: # We try to save the file, if an error is raised then we delete the record on the database diff --git a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py index 1e856953..dacb5e87 100644 --- a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py +++ b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py @@ -777,12 +777,14 @@ def test_post_report_html(self): run=False, ).json() - print(execution["id"]) - response = self.client.raw.create_report( "new_report", HTML_REPORT, execution["id"] ) + print(execution["id"]) + print(response.status_code) + print(response.json()) + self.assertEqual(response.status_code, 201) return response From 485315988cde969a090f8a6cbcc5f93fa5d033fe Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Wed, 17 Jul 2024 09:29:41 +0200 Subject: [PATCH 06/14] More debugging --- cornflow-server/cornflow/endpoints/reports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cornflow-server/cornflow/endpoints/reports.py b/cornflow-server/cornflow/endpoints/reports.py index 85f2f956..9f42dcdc 100644 --- a/cornflow-server/cornflow/endpoints/reports.py +++ b/cornflow-server/cornflow/endpoints/reports.py @@ -117,7 +117,7 @@ def post(self, **kwargs): report.save() except Exception as error: current_app.logger.error(error) - raise InvalidUsage("Error on POST report") + raise InvalidUsage(error=error) try: # We try to save the file, if an error is raised then we delete the record on the database From 3529616b0e1b6e5a8560bcb8bfe18d0866e7ba4a Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Wed, 17 Jul 2024 09:41:45 +0200 Subject: [PATCH 07/14] Modified the way the error is built to be able to decode it as a JSON --- cornflow-server/cornflow/endpoints/reports.py | 2 +- cornflow-server/cornflow/shared/exceptions.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cornflow-server/cornflow/endpoints/reports.py b/cornflow-server/cornflow/endpoints/reports.py index 9f42dcdc..1c81e705 100644 --- a/cornflow-server/cornflow/endpoints/reports.py +++ b/cornflow-server/cornflow/endpoints/reports.py @@ -117,7 +117,7 @@ def post(self, **kwargs): report.save() except Exception as error: current_app.logger.error(error) - raise InvalidUsage(error=error) + raise InvalidUsage(error=str(error)) try: # We try to save the file, if an error is raised then we delete the record on the database diff --git a/cornflow-server/cornflow/shared/exceptions.py b/cornflow-server/cornflow/shared/exceptions.py index c3f4da63..605a8e75 100644 --- a/cornflow-server/cornflow/shared/exceptions.py +++ b/cornflow-server/cornflow/shared/exceptions.py @@ -21,7 +21,10 @@ class InvalidUsage(Exception): def __init__(self, error=None, status_code=None, payload=None, log_txt=None): Exception.__init__(self, error) if error is not None: - self.error = error + if isinstance(error, Exception): + self.error = str(error) + else: + self.error = error if status_code is not None: self.status_code = status_code self.payload = payload From fba74181e82df87d409a97a6687c46dd26e2f402 Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Wed, 17 Jul 2024 09:53:08 +0200 Subject: [PATCH 08/14] The problem was that the destination folder for the files was not correct --- cornflow-server/cornflow/app.py | 3 +++ cornflow-server/cornflow/config.py | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/cornflow-server/cornflow/app.py b/cornflow-server/cornflow/app.py index d8125a93..2301af0c 100644 --- a/cornflow-server/cornflow/app.py +++ b/cornflow-server/cornflow/app.py @@ -46,6 +46,9 @@ def create_app(env_name="development", dataconn=None): :return: the application that is going to be running :class:`Flask` :rtype: :class:`Flask` """ + if os.getenv("FLASK_ENV", None) is not None: + env_name = os.getenv("FLASK_ENV") + dictConfig(log_config(app_config[env_name].LOG_LEVEL)) app = Flask(__name__) diff --git a/cornflow-server/cornflow/config.py b/cornflow-server/cornflow/config.py index a33e7d73..627b19c3 100644 --- a/cornflow-server/cornflow/config.py +++ b/cornflow-server/cornflow/config.py @@ -115,6 +115,10 @@ class Testing(DefaultConfig): AIRFLOW_PWD = os.getenv("AIRFLOW_PWD", "admin") OPEN_DEPLOYMENT = 1 LOG_LEVEL = int(os.getenv("LOG_LEVEL", 10)) + UPLOAD_FOLDER = os.getenv( + "UPLOAD_FOLDER", + os.path.abspath(os.path.join(os.path.dirname(__file__), "./static")), + ) class Production(DefaultConfig): From 8a2502a10b324681cb772b13a44bcfd3cffd91e5 Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Wed, 17 Jul 2024 12:20:45 +0200 Subject: [PATCH 09/14] Added test for the PUT of reports --- cornflow-server/cornflow/endpoints/reports.py | 74 ++++++------- .../client/cornflow_client/cornflow_client.py | 1 - .../cornflow_client/raw_cornflow_client.py | 103 +++++++++++------- .../test_raw_cornflow_integration.py | 79 +++++++++++++- 4 files changed, 172 insertions(+), 85 deletions(-) diff --git a/cornflow-server/cornflow/endpoints/reports.py b/cornflow-server/cornflow/endpoints/reports.py index 1c81e705..b5605701 100644 --- a/cornflow-server/cornflow/endpoints/reports.py +++ b/cornflow-server/cornflow/endpoints/reports.py @@ -22,7 +22,6 @@ FileError, ObjectDoesNotExist, NoPermission, - InvalidUsage, ) @@ -71,53 +70,50 @@ def post(self, **kwargs): the reference_id for the newly created report if successful) and a integer with the HTTP status code :rtype: Tuple(dict, integer) """ - try: - execution = ExecutionModel.get_one_object(idx=kwargs["execution_id"]) - if execution is None: - raise ObjectDoesNotExist("The execution does not exist") + execution = ExecutionModel.get_one_object(idx=kwargs["execution_id"]) - if "file" not in request.files: - return {"message": "No file part"}, 400 + if execution is None: + raise ObjectDoesNotExist("The execution does not exist") - file = request.files["file"] - filename = secure_filename(file.filename) - filename_extension = filename.split(".")[-1] + if "file" not in request.files: + return {"message": "No file part"}, 400 - if filename_extension not in current_app.config["ALLOWED_EXTENSIONS"]: - return { - "message": f"Invalid file extension. " - f"Valid extensions are: {current_app.config['ALLOWED_EXTENSIONS']}" - }, 400 + file = request.files["file"] + filename = secure_filename(file.filename) + filename_extension = filename.split(".")[-1] - my_directory = f"{current_app.config['UPLOAD_FOLDER']}/{execution.id}" + if filename_extension not in current_app.config["ALLOWED_EXTENSIONS"]: + return { + "message": f"Invalid file extension. " + f"Valid extensions are: {current_app.config['ALLOWED_EXTENSIONS']}" + }, 400 - # we create a directory for the execution - if not os.path.exists(my_directory): - current_app.logger.info(f"Creating directory {my_directory}") - os.mkdir(my_directory) + my_directory = f"{current_app.config['UPLOAD_FOLDER']}/{execution.id}" - report_name = f"{secure_filename(kwargs['name'])}.{filename_extension}" + # we create a directory for the execution + if not os.path.exists(my_directory): + current_app.logger.info(f"Creating directory {my_directory}") + os.mkdir(my_directory) - save_path = os.path.normpath(os.path.join(my_directory, report_name)) + report_name = f"{secure_filename(kwargs['name'])}.{filename_extension}" - if "static" not in save_path or ".." in save_path: - raise NoPermission("Invalid file name") + save_path = os.path.normpath(os.path.join(my_directory, report_name)) - report = ReportModel( - { - "name": kwargs["name"], - "file_url": save_path, - "execution_id": kwargs["execution_id"], - "user_id": execution.user_id, - "description": kwargs.get("description", ""), - } - ) + if "static" not in save_path or ".." in save_path: + raise NoPermission("Invalid file name") - report.save() - except Exception as error: - current_app.logger.error(error) - raise InvalidUsage(error=str(error)) + report = ReportModel( + { + "name": kwargs["name"], + "file_url": save_path, + "execution_id": kwargs["execution_id"], + "user_id": execution.user_id, + "description": kwargs.get("description", ""), + } + ) + + report.save() try: # We try to save the file, if an error is raised then we delete the record on the database @@ -127,7 +123,7 @@ def post(self, **kwargs): except Exception as error: report.delete() current_app.logger.error(error) - raise FileError + raise FileError(error=str(error)) class ReportDetailsEndpointBase(BaseMetaResource): @@ -181,7 +177,7 @@ def put(self, idx, **data): :rtype: Tuple(dict, integer) """ current_app.logger.info(f"User {self.get_user()} edits report {idx}") - return self.put_detail(data, user=self.get_user(), idx=idx) + return self.put_detail(data, user_id=self.get_user_id(), idx=idx) @doc(description="Delete a report", tags=["Reports"], inherit=False) @authenticate(auth_class=Auth()) diff --git a/libs/client/cornflow_client/cornflow_client.py b/libs/client/cornflow_client/cornflow_client.py index 178943f6..ea8aea81 100644 --- a/libs/client/cornflow_client/cornflow_client.py +++ b/libs/client/cornflow_client/cornflow_client.py @@ -1,7 +1,6 @@ from .raw_cornflow_client import RawCornFlow, CornFlowApiError # TODO: review the standard calls for the reports. -# TODO: modify the headers on the calls that require a file. # TODO: have the download report method to receive the path to save it on the local machine. diff --git a/libs/client/cornflow_client/raw_cornflow_client.py b/libs/client/cornflow_client/raw_cornflow_client.py index 2d4be87f..bb07bc08 100644 --- a/libs/client/cornflow_client/raw_cornflow_client.py +++ b/libs/client/cornflow_client/raw_cornflow_client.py @@ -1,5 +1,5 @@ """ - +Code for the main class to interact to cornflow from python code. """ import logging as log @@ -11,6 +11,45 @@ import requests +def ask_token(func: callable): + @wraps(func) + def wrapper(self, *args, **kwargs): + if not self.token: + raise CornFlowApiError("Need to login first!") + return func(self, *args, **kwargs) + + return wrapper + + +def log_call(func: callable): + @wraps(func) + def wrapper(*args, **kwargs): + result = func(*args, **kwargs) + log.debug(result.json()) + return result + + return wrapper + + +def prepare_encoding(func: callable): + @wraps(func) + def wrapper(*args, **kwargs): + encoding = kwargs.get("encoding", "br") + if encoding not in [ + "gzip", + "compress", + "deflate", + "br", + "identity", + ]: + encoding = "br" + kwargs["encoding"] = encoding + result = func(*args, **kwargs) + return result + + return wrapper + + class RawCornFlow(object): """ Base class to access cornflow-server @@ -20,42 +59,6 @@ def __init__(self, url, token=None): self.url = url self.token = token - def ask_token(func): - @wraps(func) - def wrapper(self, *args, **kwargs): - if not self.token: - raise CornFlowApiError("Need to login first!") - return func(self, *args, **kwargs) - - return wrapper - - def log_call(func): - @wraps(func) - def wrapper(*args, **kwargs): - result = func(*args, **kwargs) - log.debug(result.json()) - return result - - return wrapper - - def prepare_encoding(func): - @wraps(func) - def wrapper(*args, **kwargs): - encoding = kwargs.get("encoding", "br") - if encoding not in [ - "gzip", - "compress", - "deflate", - "br", - "identity", - ]: - encoding = "br" - kwargs["encoding"] = encoding - result = func(*args, **kwargs) - return result - - return wrapper - # def expect_201(func): # return partial(expect_status, status=201) # @@ -352,7 +355,7 @@ def create_execution( :param str instance_id: id for the instance :param str name: name for the execution :param str description: description of the execution - :param dict config: execution configuration + :param dict config: configuration for the execution :param str schema: name of the problem to solve :param str encoding: the type of encoding used in the call. Defaults to 'br' :param bool run: if the execution should be run or not @@ -542,6 +545,7 @@ def create_report(self, name, filename, execution_id, encoding=None, **kwargs): @ask_token @prepare_encoding def get_one_report(self, reference_id, folder_destination, encoding=None): + """""" result = self.get_api_for_id(api="report", id=reference_id, encoding=encoding) content = result.content @@ -555,6 +559,25 @@ def get_one_report(self, reference_id, folder_destination, encoding=None): return result + @ask_token + @log_call + @prepare_encoding + def delete_one_report(self, reference_id, encoding=None): + """""" + return self.delete_api_for_id(api="report", id=reference_id, encoding=encoding) + + @ask_token + @log_call + @prepare_encoding + def put_one_report(self, reference_id, payload, encoding=None): + """ + + """ + return self.put_api_for_id( + api="report", id=reference_id, payload=payload, encoding=encoding + ) + + @ask_token @prepare_encoding def write_instance_checks(self, instance_id, encoding=None, **kwargs): @@ -934,7 +957,7 @@ def create_deployed_dag( encoding=None, ): if name is None: - return {"error": "No dag name was given"} + raise CornFlowApiError("No dag name was given") payload = dict( id=name, description=description, @@ -1017,7 +1040,7 @@ def group_variables_by_name(_vars, names_list, **kwargs): # 2. key can be a tuple or a single string. # 3. if a tuple, they can be an integer or a string. # - # it dos not permit the nested dictionary format of variables + # it does not permit the nested dictionary format of variables # we copy it because we will be taking out already seen variables _vars = dict(_vars) __vars = {k: {} for k in names_list} diff --git a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py index dacb5e87..c4741ba4 100644 --- a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py +++ b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py @@ -617,6 +617,11 @@ def test_get_all_schemas(self): for schema in PUBLIC_DAGS: self.assertIn(schema, read_schemas) + def test_log_in_first(self): + client = CornFlow(url="http://127.0.0.1:5050/") + + self.assertRaises(CornFlowApiError, client.raw.get_all_instances) + class TestRawCornflowClientAdmin(TestCase): def setUp(self): @@ -758,6 +763,19 @@ def test_post_deployed_dag(self): self.assertEqual("test_dag_2", response["id"]) self.assertEqual("test_dag_2_description", response["description"]) + def test_raises_post_deployed_dag(self): + self.assertRaises( + CornFlowApiError, + self.client.raw.create_deployed_dag, + name=None, + description="test_dag_2_description", + instance_schema=dict(), + instance_checks_schema=dict(), + solution_schema=dict(), + solution_checks_schema=dict(), + config_schema=dict(), + ) + def test_post_report_html(self): client = CornFlow(url="http://127.0.0.1:5050/") _ = client.login("user", "UserPassword1!") @@ -781,17 +799,12 @@ def test_post_report_html(self): "new_report", HTML_REPORT, execution["id"] ) - print(execution["id"]) - print(response.status_code) - print(response.json()) - self.assertEqual(response.status_code, 201) return response def test_get_one_report(self): response = self.test_post_report_html() - print(response.json()) report_id = response.json()["id"] client = CornFlow(url="http://127.0.0.1:5050/") @@ -814,3 +827,59 @@ def test_get_one_report(self): # remove file from TEST_FOLDER os.remove(os.path.join(TEST_FOLDER, "new_report.html")) + + def test_get_all_reports(self): + report_1 = self.test_post_report_html().json()["id"] + report_2 = self.test_post_report_html().json()["id"] + + client = CornFlow(url="http://127.0.0.1:5050/") + _ = client.login("user", "UserPassword1!") + + response = client.raw.get_reports() + + self.assertEqual(response.status_code, 200) + self.assertGreaterEqual(len(response.json()), 2) + + client.raw.delete_one_report(reference_id=report_1) + client.raw.delete_one_report(reference_id=report_2) + + def test_put_one_report(self): + response = self.test_post_report_html() + report_id = response.json()["id"] + + client = CornFlow(url="http://127.0.0.1:5050/") + _ = client.login("user", "UserPassword1!") + + payload = {"name": "new_name", "description": "some_description"} + + response = client.raw.put_one_report(reference_id=report_id, payload=payload) + + self.assertEqual(response.status_code, 200) + + new_report = client.raw.get_one_report(reference_id=report_id) + + self.assertEqual(new_report.json()["name"], paylaod["name"]) + self.assertEqual(new_report.json()["description"], payload["description"]) + self.assertNotEqual(new_report.json()["name"], "new_report") + self.assertNotEqual(new_report.json()["description"], "") + + delete = client.raw.delete_one_report(reference_id=report_id) + self.assertEqual(delete.status_code, 200) + + def test_delete_one_report(self): + response = self.test_post_report_html() + report_id = response.json()["id"] + + client = CornFlow(url="http://127.0.0.1:5050/") + _ = client.login("user", "UserPassword1!") + + reports_before = client.raw.get_reports() + + self.assertEqual(reports_before.status_code, 200) + + response = client.raw.delete_one_report(reference_id=report_id) + self.assertEqual(response.status_code, 200) + + reports_after = client.raw.get_reports() + + self.assertLess(len(reports_after.json()), len(reports_before.json())) From a3e58716a6403ab07d4945e3deecd4f96b7d82d0 Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Wed, 17 Jul 2024 12:48:42 +0200 Subject: [PATCH 10/14] Changed the way get one report reports back the name of the report and the description --- cornflow-server/cornflow/endpoints/reports.py | 35 +++++++++++++++++-- cornflow-server/cornflow/models/reports.py | 4 +++ .../cornflow/tests/unit/test_reports.py | 21 +++++++++++ .../test_raw_cornflow_integration.py | 8 ++--- 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/cornflow-server/cornflow/endpoints/reports.py b/cornflow-server/cornflow/endpoints/reports.py index b5605701..b2d46421 100644 --- a/cornflow-server/cornflow/endpoints/reports.py +++ b/cornflow-server/cornflow/endpoints/reports.py @@ -154,7 +154,7 @@ def get(self, idx): :rtype: Tuple(dict, integer) """ current_app.logger.info(f"User {self.get_user()} gets details of report {idx}") - report = self.get_detail(user_id=self.get_user_id(), idx=idx) + report = self.get_detail(user=self.get_user(), idx=idx) if report is None: raise ObjectDoesNotExist @@ -162,7 +162,10 @@ def get(self, idx): file = f"{report.name}{file}" directory = directory[:-1] - return send_from_directory(directory, file) + response = send_from_directory(directory, file) + response.headers["File-Description"] = report.description + response.headers["File-Name"] = report.name + return response @doc(description="Edit a report", tags=["Reports"], inherit=False) @authenticate(auth_class=Auth()) @@ -177,7 +180,33 @@ def put(self, idx, **data): :rtype: Tuple(dict, integer) """ current_app.logger.info(f"User {self.get_user()} edits report {idx}") - return self.put_detail(data, user_id=self.get_user_id(), idx=idx) + + report = self.get_detail(user=self.get_user(), idx=idx) + + try: + if report.name != data["name"]: + directory, file = report.file_url.split(report.name) + + new_location = ( + f"{os.path.join(directory, secure_filename(data['name']))}{file}" + ) + old_location = report.file_url + + current_app.logger.debug(f"Old location: {old_location}") + current_app.logger.debug(f"New location: {new_location}") + + os.rename(old_location, new_location) + data["file_url"] = new_location + + except Exception as error: + current_app.logger.error(error) + return {"error": "Error moving file"}, 400 + + report.update(data) + + report.save() + + return {"message": "Updated correctly"}, 200 @doc(description="Delete a report", tags=["Reports"], inherit=False) @authenticate(auth_class=Auth()) diff --git a/cornflow-server/cornflow/models/reports.py b/cornflow-server/cornflow/models/reports.py index 73ef15c8..d1eaa218 100644 --- a/cornflow-server/cornflow/models/reports.py +++ b/cornflow-server/cornflow/models/reports.py @@ -53,6 +53,10 @@ def user_id(self): """ return db.Column(db.Integer, db.ForeignKey("users.id"), nullable=False) + @declared_attr + def user(self): + return db.relationship("UserModel") + def __init__(self, data: dict): super().__init__() self.user_id = data.get("user_id") diff --git a/cornflow-server/cornflow/tests/unit/test_reports.py b/cornflow-server/cornflow/tests/unit/test_reports.py index a142f93c..945e1aad 100644 --- a/cornflow-server/cornflow/tests/unit/test_reports.py +++ b/cornflow-server/cornflow/tests/unit/test_reports.py @@ -187,6 +187,27 @@ def test_get_one_report(self): self.assertEqual(200, response.status_code) self.assertEqual(content, file) + def test_modify_report(self): + item = self.test_new_report_html() + + payload = {"name": "new_name", "description": "some_description"} + + response = self.client.put( + f"{self.url}{item['id']}/", + headers=self.get_header_with_auth(self.token), + json=payload, + ) + + self.assertEqual(response.status_code, 200) + + response = self.client.get( + f"{self.url}{item['id']}/", headers=self.get_header_with_auth(self.token) + ) + + self.assertEqual(200, response.status_code) + self.assertEqual("new_name", dict(response.headers)["File-Name"]) + self.assertEqual("some_description", dict(response.headers)["File-Description"]) + def test_delete_report(self): item = self.test_new_report_html() response = self.client.delete( diff --git a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py index c4741ba4..90621e96 100644 --- a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py +++ b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py @@ -858,10 +858,10 @@ def test_put_one_report(self): new_report = client.raw.get_one_report(reference_id=report_id) - self.assertEqual(new_report.json()["name"], paylaod["name"]) - self.assertEqual(new_report.json()["description"], payload["description"]) - self.assertNotEqual(new_report.json()["name"], "new_report") - self.assertNotEqual(new_report.json()["description"], "") + self.assertEqual(new_report.headers["File-Name"], payload["name"]) + self.assertEqual(new_report.headers["File-Description"], payload["description"]) + self.assertNotEqual(new_report.headers["File-Name"], "new_report") + self.assertNotEqual(new_report.headers["File-Description"], "") delete = client.raw.delete_one_report(reference_id=report_id) self.assertEqual(delete.status_code, 200) From f955e62db5d0b8f04a3598f71c8de3287c6c25e1 Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Wed, 17 Jul 2024 12:50:10 +0200 Subject: [PATCH 11/14] Fixed status code on failing test --- cornflow-server/cornflow/tests/unit/test_reports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cornflow-server/cornflow/tests/unit/test_reports.py b/cornflow-server/cornflow/tests/unit/test_reports.py index 945e1aad..abab0083 100644 --- a/cornflow-server/cornflow/tests/unit/test_reports.py +++ b/cornflow-server/cornflow/tests/unit/test_reports.py @@ -158,7 +158,7 @@ def test_new_report_no_execution(self): ), ) - self.assertEqual(400, response.status_code) + self.assertEqual(404, response.status_code) self.assertTrue("error" in response.json) def test_get_no_reports(self): From c698a1743581103e48391266ec045ba6a2171ed3 Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Wed, 17 Jul 2024 12:57:30 +0200 Subject: [PATCH 12/14] Fixed error on client test --- .../tests/integration/test_raw_cornflow_integration.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py index 90621e96..f0a32fae 100644 --- a/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py +++ b/libs/client/cornflow_client/tests/integration/test_raw_cornflow_integration.py @@ -856,7 +856,9 @@ def test_put_one_report(self): self.assertEqual(response.status_code, 200) - new_report = client.raw.get_one_report(reference_id=report_id) + new_report = client.raw.get_one_report( + reference_id=report_id, folder_destination=TEST_FOLDER + ) self.assertEqual(new_report.headers["File-Name"], payload["name"]) self.assertEqual(new_report.headers["File-Description"], payload["description"]) From 35913d5654cf0a4a157cf8e58afdda6ef7230565 Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Wed, 17 Jul 2024 13:08:12 +0200 Subject: [PATCH 13/14] Added tests for base client --- .../client/cornflow_client/cornflow_client.py | 17 ++- .../integration/test_cornflow_integration.py | 108 +++++++++++++++++- 2 files changed, 121 insertions(+), 4 deletions(-) diff --git a/libs/client/cornflow_client/cornflow_client.py b/libs/client/cornflow_client/cornflow_client.py index ea8aea81..3f2df489 100644 --- a/libs/client/cornflow_client/cornflow_client.py +++ b/libs/client/cornflow_client/cornflow_client.py @@ -21,7 +21,11 @@ def __init__(self, url, token=None): ) self.create_report = self.expect_status(self.raw.create_report, 201) self.get_reports = self.expect_status(self.raw.get_reports, 200) - self.get_one_report = self.expect_status(self.raw.get_one_report, 200) + self.get_one_report = self.expect_status( + self.raw.get_one_report, 200, json=False + ) + self.put_one_report = self.expect_status(self.raw.put_one_report, 200) + self.delete_one_report = self.expect_status(self.raw.delete_one_report, 200) self.create_instance_data_check = self.expect_status( self.raw.create_instance_data_check, 201 @@ -91,10 +95,13 @@ def token(self, token): self.raw.token = token @staticmethod - def expect_status(func, expected_status=None): + def expect_status(func, expected_status=None, json=True): """ Gets the response of the call and raise an exception if the status of the response is not the expected + + The response of the call is the json in the body for those calls that are application/json + For the calls that are form/data the response of the call is the content and the headers """ def decorator(*args, **kwargs): @@ -103,7 +110,11 @@ def decorator(*args, **kwargs): raise CornFlowApiError( f"Expected a code {expected_status}, got a {response.status_code} error instead: {response.text}" ) - return response.json() + + if json: + return response.json() + else: + return response.content, response.headers return decorator diff --git a/libs/client/cornflow_client/tests/integration/test_cornflow_integration.py b/libs/client/cornflow_client/tests/integration/test_cornflow_integration.py index 742d553b..d78e6e4d 100644 --- a/libs/client/cornflow_client/tests/integration/test_cornflow_integration.py +++ b/libs/client/cornflow_client/tests/integration/test_cornflow_integration.py @@ -14,7 +14,12 @@ from cornflow_client import CornFlow, CornFlowApiError from cornflow_client.constants import STATUS_OPTIMAL, STATUS_NOT_SOLVED, STATUS_QUEUED from cornflow_client.schema.tools import get_pulp_jsonschema -from cornflow_client.tests.const import PUBLIC_DAGS, PULP_EXAMPLE +from cornflow_client.tests.const import ( + PUBLIC_DAGS, + PULP_EXAMPLE, + HTML_REPORT, + TEST_FOLDER, +) # Constants path_to_tests_dir = os.path.dirname(os.path.abspath(__file__)) @@ -665,3 +670,104 @@ def test_post_deployed_dag(self): self.assertIn(item, response.keys()) self.assertEqual("test_dag", response["id"]) self.assertEqual("test_dag_description", response["description"]) + + def test_post_report_html(self): + client = CornFlow(url="http://127.0.0.1:5050/") + _ = client.login("user", "UserPassword1!") + + data = _load_file(PULP_EXAMPLE) + + instance = client.create_instance(data, "test_example", "test_description") + + execution = client.create_execution( + instance_id=instance["id"], + config={"solver": "PULP_CBC_CMD", "timeLimit": 60}, + name="test_execution", + description="execution_description", + schema="solve_model_dag", + run=False, + ) + + response = self.client.create_report("new_report", HTML_REPORT, execution["id"]) + + self.assertEqual(response["execution_id"], execution["id"]) + + return response + + def test_get_one_report(self): + response = self.test_post_report_html() + report_id = response["id"] + + client = CornFlow(url="http://127.0.0.1:5050/") + _ = client.login("user", "UserPassword1!") + + content, headers = client.get_one_report( + reference_id=report_id, folder_destination=TEST_FOLDER + ) + + self.assertEqual(headers["File-Name"], response["name"]) + self.assertEqual(headers["File-Description"], response["description"]) + + # read from TEST FOLDER + with open(os.path.join(TEST_FOLDER, "new_report.html"), "r") as f: + file = f.read() + + # read from test/data folder + with open(HTML_REPORT, "r") as f: + file_2 = f.read() + + self.assertEqual(file, file_2) + + # remove file from TEST_FOLDER + os.remove(os.path.join(TEST_FOLDER, "new_report.html")) + + def test_get_all_reports(self): + report_1 = self.test_post_report_html()["id"] + report_2 = self.test_post_report_html()["id"] + + client = CornFlow(url="http://127.0.0.1:5050/") + _ = client.login("user", "UserPassword1!") + + response = client.get_reports() + + self.assertGreaterEqual(len(response), 2) + + client.delete_one_report(reference_id=report_1) + client.delete_one_report(reference_id=report_2) + + def test_put_one_report(self): + response = self.test_post_report_html() + report_id = response["id"] + + client = CornFlow(url="http://127.0.0.1:5050/") + _ = client.login("user", "UserPassword1!") + + payload = {"name": "new_name", "description": "some_description"} + + _ = client.put_one_report(reference_id=report_id, payload=payload) + + content, headers = client.get_one_report( + reference_id=report_id, folder_destination=TEST_FOLDER + ) + + self.assertEqual(headers["File-Name"], payload["name"]) + self.assertEqual(headers["File-Description"], payload["description"]) + self.assertNotEqual(headers["File-Name"], "new_report") + self.assertNotEqual(headers["File-Description"], "") + + _ = client.delete_one_report(reference_id=report_id) + + def test_delete_one_report(self): + response = self.test_post_report_html() + report_id = response["id"] + + client = CornFlow(url="http://127.0.0.1:5050/") + _ = client.login("user", "UserPassword1!") + + reports_before = client.get_reports() + + _ = client.delete_one_report(reference_id=report_id) + + reports_after = client.get_reports() + + self.assertLess(len(reports_after), len(reports_before)) From e4bdf9ecd9f3c454f556de508b28b6d5ca733314 Mon Sep 17 00:00:00 2001 From: Guillermo Gonzalez-Santander Date: Wed, 17 Jul 2024 13:18:56 +0200 Subject: [PATCH 14/14] Added some typing --- .../cornflow_client/raw_cornflow_client.py | 87 ++++++++++++++----- 1 file changed, 63 insertions(+), 24 deletions(-) diff --git a/libs/client/cornflow_client/raw_cornflow_client.py b/libs/client/cornflow_client/raw_cornflow_client.py index bb07bc08..ec1273fc 100644 --- a/libs/client/cornflow_client/raw_cornflow_client.py +++ b/libs/client/cornflow_client/raw_cornflow_client.py @@ -6,9 +6,11 @@ import os import re from functools import wraps +from typing import Union, Dict from urllib.parse import urljoin import requests +from requests import Response def ask_token(func: callable): @@ -55,7 +57,7 @@ class RawCornFlow(object): Base class to access cornflow-server """ - def __init__(self, url, token=None): + def __init__(self, url: str, token=None): self.url = url self.token = token @@ -67,25 +69,25 @@ def __init__(self, url, token=None): def api_for_id( self, - api, - id=None, - method="GET", - post_url=None, - query_args=None, - encoding=None, + api: str, + id: Union[str, int] = None, + method: str = "GET", + post_url: str = None, + query_args: Dict = None, + encoding: str = None, **kwargs, - ): + ) -> Response: """ - :param api: the resource in the server + :param str api: the resource in the server :param id: the id of the particular object - :param method: HTTP method to apply - :param post_url: optional action to apply - :param query_args: query arguments for the request - :param encoding: optional string with the type of encoding, if it is not specified it uses br encoding, + :param str method: HTTP method to apply + :param str post_url: optional action to apply + :param Dict query_args: query arguments for the request + :param str encoding: optional string with the type of encoding, if it is not specified it uses br encoding, options are: gzip, compress, deflate, br or identity :param kwargs: other arguments to requests.request - :return: requests.request + :return: :class:`requests.Response` """ if api[0] == "/" and self.url[-1] == "/": api = api[1:] @@ -125,7 +127,9 @@ def api_for_id( **kwargs, ) - def get_api(self, api, method="GET", encoding=None, **kwargs): + def get_api( + self, api: str, method: str = "GET", encoding: str = None, **kwargs + ) -> Response: return requests.request( method=method, url=urljoin(self.url, api) + "/", @@ -138,7 +142,14 @@ def get_api(self, api, method="GET", encoding=None, **kwargs): @ask_token @prepare_encoding - def get_api_for_id(self, api, id=None, post_url=None, encoding=None, **kwargs): + def get_api_for_id( + self, + api: str, + id: Union[str, id] = None, + post_url: str = None, + encoding: str = None, + **kwargs, + ) -> Response: """ api_for_id with a GET request """ @@ -506,8 +517,8 @@ def write_solution(self, execution_id, encoding=None, **kwargs): Edits an execution :param str execution_id: id for the execution - :param kwargs: optional data to edit :param str encoding: the type of encoding used in the call. Defaults to 'br' + :param kwargs: optional data to edit """ return self.put_api_for_id( "dag/", id=execution_id, encoding=encoding, payload=kwargs @@ -517,7 +528,14 @@ def write_solution(self, execution_id, encoding=None, **kwargs): @log_call @prepare_encoding def get_reports(self, params=None, encoding=None): - """ """ + """ + Gets all reports for a given user + + :param dict params: optional filters + :param str encoding: the type of encoding used in the call. Defaults to 'br' + :return: the response object + :rtype: :class:`Response` + """ return self.get_api("report", params=params, encoding=encoding) @ask_token @@ -529,8 +547,8 @@ def create_report(self, name, filename, execution_id, encoding=None, **kwargs): :param str execution_id: id for the execution :param str name: the name of the report :param file filename: the file object with the report (e.g., open(REPORT_FILE_PATH, "rb")) - :param kwargs: optional data to write (description) :param str encoding: the type of encoding used in the call. Defaults to 'br' + :param kwargs: optional data to write (description) """ with open(filename, "rb") as _file: result = self.create_api( @@ -544,8 +562,18 @@ def create_report(self, name, filename, execution_id, encoding=None, **kwargs): @ask_token @prepare_encoding - def get_one_report(self, reference_id, folder_destination, encoding=None): - """""" + def get_one_report( + self, reference_id, folder_destination, encoding=None + ) -> Response: + """ + Gets one specific report and downloads it to disk + + :param int reference_id: id of the report to download + :param str folder_destination: Path on the local system where to save the downloaded report + :param str encoding: the type of encoding used in the call. Defaults to 'br' + :return: the response object + :rtype: :class:`Response` + """ result = self.get_api_for_id(api="report", id=reference_id, encoding=encoding) content = result.content @@ -563,21 +591,32 @@ def get_one_report(self, reference_id, folder_destination, encoding=None): @log_call @prepare_encoding def delete_one_report(self, reference_id, encoding=None): - """""" + """ + Deletes a report + + :param int reference_id: id of the report to download + :param str encoding: the type of encoding used in the call. Defaults to 'br' + :return: the response object + :rtype: :class:`Response` + """ return self.delete_api_for_id(api="report", id=reference_id, encoding=encoding) @ask_token @log_call @prepare_encoding - def put_one_report(self, reference_id, payload, encoding=None): + def put_one_report(self, reference_id, payload, encoding=None) -> Response: """ + Edits one specific report and downloads it to disk + :param int reference_id: id of the report to download + :param str encoding: the type of encoding used in the call. Defaults to 'br' + :return: the response object + :rtype: :class:`Response` """ return self.put_api_for_id( api="report", id=reference_id, payload=payload, encoding=encoding ) - @ask_token @prepare_encoding def write_instance_checks(self, instance_id, encoding=None, **kwargs):