From 966e45b7d70c24939a9d91d33d342811b392571d Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Fri, 19 Jan 2024 13:45:05 +0100 Subject: [PATCH] Update Readme: first draft, log error instead of raising exception on failed upload, cleanup tests a bit --- README.md | 41 ++++++++++++++----- daily_read/__main__.py | 13 +++++-- daily_read/order_portal.py | 11 ++++-- tests/conftest.py | 38 +++++++----------- tests/test_daily_report.py | 4 +- tests/test_ngi_data.py | 2 - tests/test_order_portal.py | 80 ++++++++++++++++++++++++-------------- 7 files changed, 114 insertions(+), 75 deletions(-) diff --git a/README.md b/README.md index c204632..1b0ec5d 100644 --- a/README.md +++ b/README.md @@ -6,14 +6,24 @@ A utility to generate and upload automatic progress reports for NGI Sweden. ## How it works -- The script first fetches data from the appropriate NGI source, i.e. statusdb for Stockholm. +- The script first fetches data from the appropriate NGI source, i.e. statusdb for Stockholm. Only projects with orders with a signed contract wil be fetched - The data corresponding to each project will then be saved in a small data file (json, yaml or csv perhaps) on disk. - Git will be used to track the directory where these files are kept (between runs of the script). - Git status (inside python) will be used to check which projects has changes in their data since the last run and those projects will be selected. -- For each orderer, fetch all their recent projects from Order Portal. A report will be generated with potentially several projects. -- Reports are uploaded to each project. -- We need to make sure the reports are transparent about timestamps when it was last updated - Javascript? -- If problems to upload to a project? +- From the projects modified, a list of their associated orderers is created. +- For each unique orderer in the list, all projects are fetched from Order Portal. These are then filtered on the following criteria + + - All orders which are not in the data fetched from the NGI source are skipped. + - All closed orders with close dates within 5 days of the cutoff date (default 30 days) will have their reports hidden. + - All closed orders closed more than 5 days before the cutoff date are assumed to have their reports hidden and will be skipped. + - All orders which are associated to a project NGI started to process but then aborted will have their reports hidden. + +- A report with all active projects is generated for each unique orderer. +- Reports are uploaded to all orders accepted by NGI which have active projects or are marked to have their reports hidden. +- As each report is uploaded, the file of the corresponding project is staged for commit in the git repo. +- After all reports in the current run are uploaded, all staged files are committed in the git repo. +- The reports have timestamps which indicate when it was last updated +- If there are problems to upload to an order - Report to error log (cron will email this) - Do not stage these changes, will make sure that the orderer is re-tried next time. - Continue with next project @@ -26,14 +36,15 @@ Also see diagram below: ## Usage ```bash -# Generate reports and save in a local git repository (location is given by configuration variable) and commit changes with a timestamp message +# Generate reports for all orderers and save them in a local directory. Project changes are saved in a local git repository (location is given by configuration variable) and committed with a timestamp message daily_read generate all -# Generate report for single orderer, +# Generate and upload reports for all orderers to order portal. They will not be saved locally +daily_read generate all --upload + +# Generate report for single orderer and save it to local directory daily_read generate single --project -# Generate and upload -daily_read generate all --upload ``` To generate and upload reports for a single user(or a list of users), their name(s) can be entered in a text file and provided to the environment variable `DAILY_READ_USERS_LIST_LOCATION`. @@ -42,6 +53,18 @@ To generate and upload reports for a single user(or a list of users), their name Configuration is dealt with via environment variables. Simplest way to set it up is to retrieve a `.env` file based on the `.env.example` provided in the repo. Environment variables which are not set have default variables in `daily_read/config.py`. +- `DAILY_READ_ORDER_PORTAL_URL` (str) : Order portal URL +- `DAILY_READ_ORDER_PORTAL_API_KEY` (str) : Order portal API key +- `DAILY_READ_REPORTS_LOCATION` (str) : Local disk location to save generated reports +- `DAILY_READ_DATA_LOCATION` (str) : Local disk location for data git repository +- `DAILY_READ_LOG_LOCATION` (str) : Local disk location to save log output +- `DAILY_READ_STHLM_STATUSDB_URL` (str) : NGI STHLM Data source URL +- `DAILY_READ_STHLM_STATUSDB_USERNAME` (str) : NGI STHLM Data source credentials +- `DAILY_READ_STHLM_STATUSDB_PASSWORD` (str) : NGI STHLM Data source credentials +- `DAILY_READ_FETCH_FROM_NGIS` (str) : Flag to turn on data fetching from NGI STHLM +- `DAILY_READ_SNPSEQ_URL` (str) : NGI STHLM Data source URL +- `DAILY_READ_USERS_LIST_LOCATION` (str) : Path on local disk to orderer list file (.txt) to send reports to, can be empty. The file would contain a single column of orderer email addresses. + ## Developer note ### Formatting with Black and Prettier diff --git a/daily_read/__main__.py b/daily_read/__main__.py index 88d81e0..2a25ad7 100644 --- a/daily_read/__main__.py +++ b/daily_read/__main__.py @@ -96,13 +96,18 @@ def generate_all(upload=False, develop=False): # Publish reports for status in modified_orders[owner]["projects"].keys(): for project in modified_orders[owner]["projects"][status]: - op.upload_report_to_order_portal(report, project, "published") - op.projects_data.stage_data_for_project(project) + uploaded = False + uploaded = op.upload_report_to_order_portal(report, project, "published") + if uploaded: + op.projects_data.stage_data_for_project(project) # Hide old reports for status in modified_orders[owner]["delete_report_for"].keys(): for project in modified_orders[owner]["delete_report_for"][status]: - op.upload_report_to_order_portal("", project, "review") - op.projects_data.stage_data_for_project(project) + uploaded = False + uploaded = op.upload_report_to_order_portal("", project, "review") + if uploaded: + op.projects_data.stage_data_for_project(project) + # Commit all uploaded projects op.projects_data.commit_staged_data(f"Commit reports updates for {datetime.datetime.now()}") else: diff --git a/daily_read/order_portal.py b/daily_read/order_portal.py index 422e54f..ff7b072 100644 --- a/daily_read/order_portal.py +++ b/daily_read/order_portal.py @@ -161,6 +161,11 @@ def upload_report_to_order_portal(self, report, project, status): # TODO: check Encoded to utf-8 to display special characters properly response = requests.post(url, headers=self.headers, json=indata) - assert response.status_code == 200, (response.status_code, response.reason) - - log.info(f"Updated report for order with project id: {project.project_id}") + if response.status_code == 200: + log.info(f"Updated report for order with project id: {project.project_id}") + return True + else: + log.error( + f"Report not uploaded for order with project id: {project.project_id}\nReason: {response.status_code} {response.reason}" + ) + return False diff --git a/tests/conftest.py b/tests/conftest.py index 87b9743..e6719d6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,7 +7,7 @@ from datetime import date, timedelta -from daily_read import ngi_data, config +from daily_read import ngi_data, config, order_portal dummy_order_open = { "orderer": "dummy@dummy.se", @@ -376,18 +376,13 @@ def create_report_path(tmp_path): return create_report_path -def mocked_requests_get(*args, **kwargs): - class MockResponse: - def __init__(self, json_data, status_code): - self.json_data = json_data - self.status_code = status_code +@pytest.fixture(autouse=True) +def mocked_requests_get(monkeypatch): + """order_portal.OrderPortal._get() mocked to return {'items': [order list]}.""" + class MockResponse: def json(self): - return self.json_data - - if args[0] == "api/v1/orders": - return MockResponse( - { + return { "items": [ order_portal_resp_order_processing, order_portal_resp_order_closed, @@ -395,11 +390,12 @@ def json(self): order_portal_resp_order_processing_single_report, order_portal_resp_order_processing_to_aborted, ] - }, - 200, - ) + } + + def mock_get(*args, **kwargs): + return MockResponse() - return MockResponse(None, 404) + monkeypatch.setattr(order_portal.OrderPortal, "_get", mock_get) @pytest.fixture @@ -414,10 +410,7 @@ def mocked_statusdb_conn_rows(): "order_year": "2023", "project_id": "P123457", "project_name": "D.Dummysson_23_03", - "proj_dates": { - "2023-06-15": ["Samples Received"], - "2023-06-28": ["Reception Control finished", "Library QC finished"], - }, + "proj_dates": dummy_order_open["project_dates"], "status": "Ongoing", }, ) @@ -430,12 +423,7 @@ def mocked_statusdb_conn_rows(): "order_year": "2023", "project_id": "P123458", "project_name": "T.Dummysson_23_04", - "proj_dates": { - "2023-06-15": ["Samples Received"], - "2023-06-28": ["Reception Control finished", "Library QC finished"], - "2023-07-28": ["All Samples Sequenced"], - "2023-07-29": ["All Raw data Delivered"], - }, + "proj_dates": dummy_order_closed["project_dates"], "status": "Closed", }, ) diff --git a/tests/test_daily_report.py b/tests/test_daily_report.py index 4bf6375..f98585a 100644 --- a/tests/test_daily_report.py +++ b/tests/test_daily_report.py @@ -1,7 +1,6 @@ import datetime import os -from conftest import mocked_requests_get from unittest import mock from daily_read import daily_report, config, ngi_data, order_portal @@ -19,8 +18,7 @@ def test_write_report_to_out_dir(data_repo_full, mock_project_data_record, creat data_master.data = {order_id: mock_project_data_record("open")} op = order_portal.OrderPortal(config_values, data_master) - with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get): - op.get_orders(orderer=orderer) + op.get_orders(orderer=orderer) assert op.all_orders[0]["identifier"] == order_id modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV) diff --git a/tests/test_ngi_data.py b/tests/test_ngi_data.py index 633580a..636b6b8 100644 --- a/tests/test_ngi_data.py +++ b/tests/test_ngi_data.py @@ -6,8 +6,6 @@ from daily_read import ngi_data, config -LOGGER = logging.getLogger(__name__) - ####################################################### TESTS ######################################################### diff --git a/tests/test_order_portal.py b/tests/test_order_portal.py index 351c316..3290965 100644 --- a/tests/test_order_portal.py +++ b/tests/test_order_portal.py @@ -1,12 +1,36 @@ import base64 +import logging import pytest -from conftest import mocked_requests_get from unittest import mock from daily_read import order_portal, config, ngi_data +def test_get_and_process_orders_open_upload_fail(data_repo_full, mock_project_data_record, caplog): + orderer = "dummy@dummy.se" + order_id = "NGI123456" + config_values = config.Config() + with mock.patch("daily_read.statusdb.StatusDBSession"): + data_master = ngi_data.ProjectDataMaster(config_values) + + data_master.data = {order_id: mock_project_data_record("open")} + + op = order_portal.OrderPortal(config_values, data_master) + op.get_orders(orderer=orderer) + + assert op.all_orders[0]["identifier"] == order_id + modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV) + assert modified_orders[orderer]["projects"]["Library QC finished"][0] == data_master.data[order_id] + with mock.patch("daily_read.order_portal.requests.post") as mock_post: + mock_post.return_value.status_code = 404 + uploaded = op.upload_report_to_order_portal( + "test data", modified_orders[orderer]["projects"]["Library QC finished"][0], "published" + ) + assert not uploaded + assert f"Report not uploaded for order with project id: {order_id}\nReason: 404" in caplog.text + + def test_get_and_process_orders_open_and_upload(data_repo_full, mock_project_data_record): """Test getting and processing an open order and uploading its Project progress report and uploading the report to the Order portal""" orderer = "dummy@dummy.se" @@ -18,8 +42,7 @@ def test_get_and_process_orders_open_and_upload(data_repo_full, mock_project_dat data_master.data = {order_id: mock_project_data_record("open")} op = order_portal.OrderPortal(config_values, data_master) - with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get): - op.get_orders(orderer=orderer) + op.get_orders(orderer=orderer) assert op.all_orders[0]["identifier"] == order_id modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV) @@ -45,7 +68,7 @@ def test_get_and_process_orders_open_and_upload(data_repo_full, mock_project_dat ) -def test_get_and_process_orders_open_with_report_and_upload(data_repo_full, mock_project_data_record): +def test_get_and_process_orders_open_with_report_and_upload(data_repo_full, mock_project_data_record, caplog): """Test getting, processing an open order with an existing Project progress report and uploading the report to the Order portal""" orderer = "dummy@dummy.se" order_id = "NGI123453" @@ -56,31 +79,33 @@ def test_get_and_process_orders_open_with_report_and_upload(data_repo_full, mock data_master.data = {order_id: mock_project_data_record("open_with_report")} op = order_portal.OrderPortal(config_values, data_master) - with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get): - op.get_orders(orderer=orderer) + op.get_orders(orderer=orderer) assert op.all_orders[3]["identifier"] == order_id modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV) assert modified_orders[orderer]["projects"]["Library QC finished"][0] == data_master.data[order_id] with mock.patch("daily_read.order_portal.requests.post") as mock_post: mock_post.return_value.status_code = 200 - op.upload_report_to_order_portal( - "test data", modified_orders[orderer]["projects"]["Library QC finished"][0], "published" - ) - url = f"{config_values.ORDER_PORTAL_URL}/api/v1/report/{op.all_orders[3]['reports'][0]['iuid']}" - indata = dict( - order=order_id, - name="Project Progress", - status="published", - file=dict( - data=base64.b64encode("test data".encode()).decode("utf-8"), - filename="project_progress.html", - content_type="text/html", - ), - ) - mock_post.assert_called_once_with( - url, headers={"X-OrderPortal-API-key": config_values.ORDER_PORTAL_API_KEY}, json=indata - ) + with caplog.at_level(logging.INFO): + uploaded = op.upload_report_to_order_portal( + "test data", modified_orders[orderer]["projects"]["Library QC finished"][0], "published" + ) + url = f"{config_values.ORDER_PORTAL_URL}/api/v1/report/{op.all_orders[3]['reports'][0]['iuid']}" + indata = dict( + order=order_id, + name="Project Progress", + status="published", + file=dict( + data=base64.b64encode("test data".encode()).decode("utf-8"), + filename="project_progress.html", + content_type="text/html", + ), + ) + assert uploaded + mock_post.assert_called_once_with( + url, headers={"X-OrderPortal-API-key": config_values.ORDER_PORTAL_API_KEY}, json=indata + ) + assert f"Updated report for order with project id: {order_id}" in caplog.text def test_get_and_process_orders_open_to_aborted_with_report_and_upload(data_repo_full, mock_project_data_record): @@ -94,8 +119,7 @@ def test_get_and_process_orders_open_to_aborted_with_report_and_upload(data_repo data_master.data = {order_id: mock_project_data_record("open_to_aborted_with_report")} op = order_portal.OrderPortal(config_values, data_master) - with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get): - op.get_orders(orderer=orderer) + op.get_orders(orderer=orderer) assert op.all_orders[4]["identifier"] == order_id modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV) @@ -128,8 +152,7 @@ def test_get_and_process_orders_closed(data_repo_full, mock_project_data_record) data_master.data = {order_id: mock_project_data_record("closed")} op = order_portal.OrderPortal(config_values, data_master) - with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get): - op.get_orders(orderer=orderer) + op.get_orders(orderer=orderer) assert op.all_orders[1]["identifier"] == order_id modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV) @@ -147,8 +170,7 @@ def test_get_and_process_orders_mult_reports(data_repo_full, mock_project_data_r data_master.data = {order_id: mock_project_data_record("open")} op = order_portal.OrderPortal(config_values, data_master) - with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get): - op.get_orders(orderer=orderer) + op.get_orders(orderer=orderer) assert op.all_orders[2]["identifier"] == order_id with pytest.raises(