Skip to content

Commit 6dccb2e

Browse files
[PRMT-229] Performance Testing for Full Stitching Process (#662)
* [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - Create workflows to run script * [PRMT-229] - Rename workflows * [PRMT-229] - Add pull request parameter to workflow * [PRMT-229] - Temporarily set workflow sandbox to ndrc * [PRMT-229] - removed logging * [PRMT-229] - updated variable name to not clash * [PRMT-229] - removed no longer necessary variable * [PRMT-229] - updated variable type for patient number and file number * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] WIP * [PRMT-229] WIP * [PRMT-229] WIP * [PRMT-229] WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - copied updated bulk upload to bulk upload to verify permissions * [PRMT-229] - reverted * [PRMT-229] - copied updated_setup_bulk_upload to setup_bulk_upload * [PRMT-229] - copied setup_bulk_upload to updated_setup_bulk_upload to verify permissions * [PRMT-229] - updated to continue the test * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - WIP * [PRMT-229] - reverted code to use updated bulk upload code * [PRMT-229] - updated num-patients flag * [PRMT-229] - updated updated to reflect how it was before * [PRMT-229] - updated logic to try and upload 1 file * [PRMT-229] - added logging * [PRMT-229] - added logging * [PRMT-229] - trying to write default file * [PRMT-229] - trying to upload several files * [PRMT-229] - updated nhs number and file paths * [PRMT-229] - updated how nhs number is created * [PRMT-229] - updated how nhs number is created * [PRMT-229] - removed combi-settings * [PRMT-229] - small efficiency update * [PRMT-229] - optimized create_test_file_keys * [PRMT-229] - updated generate_nhs_number * [PRMT-229] - updated generate_nhs_number * [PRMT-229] - fixed error * [PRMT-229] - fixed error * [PRMT-229] - optimized generate_nhs_number * [PRMT-229] - testing threads for upload_lg_files_to_staging * [PRMT-229] - added metadata * [PRMT-229] - fixed metadata output * [PRMT-229] - allow file size customization * [PRMT-229] - updated pypdf dependency * [PRMT-229] - added new argument to updated_setup_bulk_upload.py * [PRMT-229] - updated how arguments are collected * [PRMT-229] - fixed typo * [PRMT-229] - testing speed with adding blank pages to pdf * [PRMT-229] - testing speed with empty bytes * [PRMT-229] - testing different way of changing file size * [PRMT-229] - removed duplication of upload * [PRMT-229] - allow file size to be float * [PRMT-229] - allow file size to be float * [PRMT-229] - cleaned a bit the code * [PRMT-70] - cleaned code * [PRMT-229] - testing run bulk upload * [PRMT-229] - testing run bulk upload * [PRMT-229] - testing run bulk upload * [PRMT-229] - testing run bulk upload * [PRMT-229] - testing run bulk upload * [PRMT-229] - testing run bulk upload * [PRMT-229] - testing run bulk upload * [PRMT-229] - testing run bulk upload * [PRMT-229] - stubbing pds * [PRMT-229] - added new env variable to try and stub pds * [PRMT-229] - added logging * [PRMT-229] - added logging * [PRMT-229] - updated mock logic * [PRMT-229] - updated file name to make it pass validation, and changed validation strict mode * [PRMT-229] - skipp validations if we want to always have good pds * [PRMT-229] - skipp validations if we want to always have good pds * [PRMT-229] - skipp validations if we want to always have good pds * [PRMT-229] - optimization when mocking pds * [PRMT-229] - removed unnecessary logging * [PRMT-229] - Added tests * [PRMT-229] - minor fix * test using number * test using number * test using number * test using number * test using number * test using number * test using number * test using number * reverted number differences in github action * deleted a method * deleted a method * removed unnused variables * fixed variable initialization * testing with all variables * testing with only STAGING_BUCKET variable * delete unused variables * testing removing copy_to_s3 method * copy_to_s3 method * removed wrong access * added STAGING_BUCKET * [PRMT-229]-addressed ticket comments * [PRMT-229]-addressed ticket comments --------- Co-authored-by: Mohammad Iqbal <[email protected]>
1 parent 9062a72 commit 6dccb2e

File tree

9 files changed

+851
-48
lines changed

9 files changed

+851
-48
lines changed
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
name: "Updated Base Bulk Upload - Execute a Bulk Upload"
2+
3+
permissions:
4+
pull-requests: write
5+
id-token: write # This is required for requesting the JWT
6+
contents: read # This is required for actions/checkout
7+
8+
on:
9+
workflow_call:
10+
inputs:
11+
sandbox:
12+
description: "Which Sandbox to push to."
13+
required: true
14+
type: "string"
15+
default: "ndr"
16+
base_branch:
17+
description: "Which Feature Branch for the Bulk Upload Script"
18+
required: false
19+
type: "string"
20+
default: "main"
21+
environment:
22+
description: "Which Environment settings to use."
23+
required: true
24+
type: "string"
25+
default: "development"
26+
num_patients:
27+
description: "How many patients to create (default = 1)"
28+
required: false
29+
type: "string"
30+
default: "1"
31+
file_count:
32+
description: "How many files per patient to generate."
33+
required: false
34+
type: "string"
35+
default: "1"
36+
file_size:
37+
description: "File size to generate."
38+
required: false
39+
type: "string"
40+
default: "1"
41+
secrets:
42+
AWS_ASSUME_ROLE:
43+
required: true
44+
45+
jobs:
46+
perform-bulk-upload:
47+
runs-on: ubuntu-latest
48+
environment: ${{ inputs.environment }}
49+
steps:
50+
- name: Configure AWS Credentials for ${{ vars.AWS_REGION }}
51+
uses: aws-actions/configure-aws-credentials@v4
52+
with:
53+
role-to-assume: ${{ secrets.AWS_ASSUME_ROLE }}
54+
role-skip-session-tagging: true
55+
aws-region: ${{ vars.AWS_REGION }}
56+
mask-aws-account-id: true
57+
58+
- name: Checkout repository
59+
uses: actions/checkout@v4
60+
61+
- name: Set up Python
62+
uses: actions/setup-python@v5
63+
with:
64+
python-version: "3.11"
65+
66+
- name: Install dependencies
67+
run: |
68+
python -m pip install --upgrade pip
69+
pip install boto3
70+
working-directory: ./tests/bulk-upload/scripts
71+
72+
- name: Setup Bulk Upload
73+
run: |
74+
python updated_setup_bulk_upload.py \
75+
--environment "${{ inputs.sandbox }}" \
76+
--delete-table \
77+
--download-data \
78+
--build-files \
79+
--num-patients "${{ inputs.num_patients }}" \
80+
--upload \
81+
--num-files "${{ inputs.file_count }}" \
82+
--file-size "${{ inputs.file_size }}" \
83+
--empty-lloydgeorge-store
84+
working-directory: ./tests/bulk-upload/scripts
85+
86+
- name: Run Bulk Upload
87+
run: |
88+
python run_bulk_upload.py \
89+
--environment "${{ inputs.sandbox }}" \
90+
--start-bulk-upload
91+
working-directory: ./tests/bulk-upload/scripts
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
name: Updated Run Bulk Upload - Test
2+
3+
permissions:
4+
pull-requests: write
5+
id-token: write # This is required for requesting the JWT
6+
contents: read # This is required for actions/checkout
7+
8+
on:
9+
workflow_dispatch:
10+
inputs:
11+
sandbox:
12+
description: "Which Sandbox to push to."
13+
required: true
14+
type: "string"
15+
default: "ndr"
16+
num_patients:
17+
description: "How many patients to create (default = 1)"
18+
required: true
19+
type: "string"
20+
default: "1"
21+
file_count:
22+
description: "How many files per patient"
23+
required: true
24+
type: "string"
25+
default: "1"
26+
file_size:
27+
description: "File size in MB"
28+
required: true
29+
type: "string"
30+
default: "1"
31+
32+
jobs:
33+
bulk_upload:
34+
uses: ./.github/workflows/updated-base-run-bulk-upload.yml
35+
with:
36+
environment: development
37+
sandbox: "${{ inputs.sandbox }}"
38+
num_patients: "${{ inputs.num_patients }}"
39+
file_count: "${{ inputs.file_count }}"
40+
file_size: "${{ inputs.file_size }}"
41+
secrets:
42+
AWS_ASSUME_ROLE: ${{ secrets.AWS_ASSUME_ROLE }}

lambdas/handlers/bulk_upload_handler.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import os
2+
13
from enums.feature_flags import FeatureFlags
24
from services.bulk_upload_service import BulkUploadService
35
from services.feature_flags_service import FeatureFlagService
@@ -25,6 +27,7 @@ def lambda_handler(event, _context):
2527
validation_strict_mode = validation_strict_mode_flag_object[
2628
FeatureFlags.LLOYD_GEORGE_VALIDATION_STRICT_MODE_ENABLED.value
2729
]
30+
pds_fhir_always_true = os.getenv("PDS_FHIR_ALWAYS_TRUE", "false").lower() == "true"
2831

2932
if validation_strict_mode:
3033
logger.info("Lloyd George validation strict mode is enabled")
@@ -39,7 +42,9 @@ def lambda_handler(event, _context):
3942
status_code=http_status_code, body=response_body, methods="GET"
4043
).create_api_gateway_response()
4144

42-
bulk_upload_service = BulkUploadService(strict_mode=validation_strict_mode)
45+
bulk_upload_service = BulkUploadService(
46+
strict_mode=validation_strict_mode, pds_fhir_always_true=pds_fhir_always_true
47+
)
4348

4449
try:
4550
bulk_upload_service.process_message_queue(event["Records"])

lambdas/services/bulk_upload_service.py

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949

5050

5151
class BulkUploadService:
52-
def __init__(self, strict_mode):
52+
def __init__(self, strict_mode, pds_fhir_always_true=False):
5353
self.dynamo_repository = BulkUploadDynamoRepository()
5454
self.sqs_repository = BulkUploadSqsRepository()
5555
self.s3_repository = BulkUploadS3Repository()
@@ -58,6 +58,7 @@ def __init__(self, strict_mode):
5858
self.unhandled_messages = []
5959
self.file_path_cache = {}
6060
self.pdf_stitching_queue_url = os.environ["PDF_STITCHING_SQS_URL"]
61+
self.pds_fhir_always_true = pds_fhir_always_true
6162

6263
def process_message_queue(self, records: list):
6364
for index, message in enumerate(records, start=1):
@@ -120,6 +121,7 @@ def handle_sqs_message(self, message: dict):
120121
raise InvalidMessageException(str(e))
121122

122123
logger.info("SQS event is valid. Validating NHS number and file names")
124+
123125
try:
124126
file_names = [
125127
os.path.basename(metadata.file_path)
@@ -134,43 +136,44 @@ def handle_sqs_message(self, message: dict):
134136
patient_ods_code = (
135137
pds_patient_details.get_ods_code_or_inactive_status_for_gp()
136138
)
137-
if not self.strict_mode:
138-
(
139-
name_validation_accepted_reason,
140-
is_name_validation_based_on_historic_name,
141-
) = validate_filename_with_patient_details_lenient(
142-
file_names, pds_patient_details
143-
)
144-
accepted_reason = self.concatenate_acceptance_reason(
145-
accepted_reason, name_validation_accepted_reason
146-
)
147-
else:
148-
is_name_validation_based_on_historic_name = (
149-
validate_filename_with_patient_details_strict(
139+
if not self.pds_fhir_always_true:
140+
if not self.strict_mode:
141+
(
142+
name_validation_accepted_reason,
143+
is_name_validation_based_on_historic_name,
144+
) = validate_filename_with_patient_details_lenient(
150145
file_names, pds_patient_details
151146
)
152-
)
153-
if is_name_validation_based_on_historic_name:
154-
accepted_reason = self.concatenate_acceptance_reason(
155-
accepted_reason, "Patient matched on historical name"
156-
)
147+
accepted_reason = self.concatenate_acceptance_reason(
148+
accepted_reason, name_validation_accepted_reason
149+
)
150+
else:
151+
is_name_validation_based_on_historic_name = (
152+
validate_filename_with_patient_details_strict(
153+
file_names, pds_patient_details
154+
)
155+
)
156+
if is_name_validation_based_on_historic_name:
157+
accepted_reason = self.concatenate_acceptance_reason(
158+
accepted_reason, "Patient matched on historical name"
159+
)
157160

158-
if not allowed_to_ingest_ods_code(patient_ods_code):
159-
raise LGInvalidFilesException("Patient not registered at your practice")
160-
patient_death_notification_status = (
161-
pds_patient_details.get_death_notification_status()
162-
)
163-
if patient_death_notification_status:
164-
deceased_accepted_reason = (
165-
f"Patient is deceased - {patient_death_notification_status.name}"
166-
)
167-
accepted_reason = self.concatenate_acceptance_reason(
168-
accepted_reason, deceased_accepted_reason
169-
)
170-
if patient_ods_code is PatientOdsInactiveStatus.RESTRICTED:
171-
accepted_reason = self.concatenate_acceptance_reason(
172-
accepted_reason, "PDS record is restricted"
161+
if not allowed_to_ingest_ods_code(patient_ods_code):
162+
raise LGInvalidFilesException(
163+
"Patient not registered at your practice"
164+
)
165+
patient_death_notification_status = (
166+
pds_patient_details.get_death_notification_status()
173167
)
168+
if patient_death_notification_status:
169+
deceased_accepted_reason = f"Patient is deceased - {patient_death_notification_status.name}"
170+
accepted_reason = self.concatenate_acceptance_reason(
171+
accepted_reason, deceased_accepted_reason
172+
)
173+
if patient_ods_code is PatientOdsInactiveStatus.RESTRICTED:
174+
accepted_reason = self.concatenate_acceptance_reason(
175+
accepted_reason, "PDS record is restricted"
176+
)
174177

175178
except (
176179
InvalidNhsNumberException,

lambdas/services/mock_pds_service.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99

1010

1111
class MockPdsApiService(PatientSearch):
12-
def __init__(self, *args, **kwargs):
12+
def __init__(self, always_pass_mock: bool = False, *args, **kwargs):
13+
self.always_pass_mock = always_pass_mock
1314
pass
1415

1516
def pds_request(self, nhs_number: str, *args, **kwargs) -> Response:
@@ -33,21 +34,24 @@ def pds_request(self, nhs_number: str, *args, **kwargs) -> Response:
3334
raise PdsErrorException("Error when requesting patient from PDS")
3435

3536
pds_patient: dict = {}
36-
37-
for result in mock_pds_results:
38-
mock_patient_nhs_number = result.get("id")
39-
if mock_patient_nhs_number == nhs_number:
40-
pds_patient = result
41-
break
42-
4337
response = Response()
38+
if self.always_pass_mock:
39+
pds_patient_index = 3
40+
pds_patient = mock_pds_results[pds_patient_index]
41+
pds_patient["id"] = nhs_number
42+
pds_patient["identifier"][0]["value"] = nhs_number
43+
else:
44+
for result in mock_pds_results:
45+
mock_patient_nhs_number = result.get("id")
46+
if mock_patient_nhs_number == nhs_number:
47+
pds_patient = result
48+
break
4449

4550
if bool(pds_patient):
4651
response.status_code = 200
4752
response._content = json.dumps(pds_patient).encode("utf-8")
4853
else:
4954
response.status_code = 404
50-
5155
return response
5256

5357
def too_many_requests_response(self) -> Response:

lambdas/utils/utilities.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ def get_pds_service() -> PatientSearch:
7171
auth_service = NhsOauthService(ssm_service)
7272
return PdsApiService(ssm_service, auth_service)
7373
else:
74-
return MockPdsApiService()
74+
return MockPdsApiService(
75+
always_pass_mock=os.getenv("PDS_FHIR_ALWAYS_TRUE") in ["True", "true"]
76+
)
7577

7678

7779
def redact_id_to_last_4_chars(str_id: str) -> str:

tests/bulk-upload/scripts/run_bulk_upload.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
import boto3
2-
import json
31
import argparse
2+
import json
3+
import sys
4+
5+
import boto3
46

57

68
def invoke_lambda(lambda_name, payload={}):
@@ -53,10 +55,17 @@ def update_lambda_environment_variables(lambda_name, new_variables):
5355

5456
bulk_upload_lambda_name = f"{args.environment}_BulkUploadLambda"
5557
search_lambda_name = f"{args.environment}_SearchPatientDetailsLambda"
56-
if args.disable_pds_stub or input("Would you like to disable the FHIR Stub: "):
58+
if args.disable_pds_stub or (
59+
sys.stdin.isatty()
60+
and input("Would you like to disable the FHIR Stub: ").lower() == "y"
61+
):
5762
new_variables = {"PDS_FHIR_IS_STUBBED": "false"}
5863
update_lambda_environment_variables(bulk_upload_lambda_name, new_variables)
5964
update_lambda_environment_variables(search_lambda_name, new_variables)
65+
else:
66+
new_variables = {"PDS_FHIR_IS_STUBBED": "true", "PDS_FHIR_ALWAYS_TRUE": "true"}
67+
update_lambda_environment_variables(bulk_upload_lambda_name, new_variables)
68+
update_lambda_environment_variables(search_lambda_name, new_variables)
6069
if args.start_bulk_upload or input(
6170
"Would you like to start the Bulk Upload Process:"
6271
):

0 commit comments

Comments
 (0)