Skip to content

VED-1235 Lambda-to-mock-PDS-in-Ref #1427

Draft
amarauzoma wants to merge 24 commits intomasterfrom
VED-1235-Lambda-to-mock-PDS-in-Ref
Draft

VED-1235 Lambda-to-mock-PDS-in-Ref #1427
amarauzoma wants to merge 24 commits intomasterfrom
VED-1235-Lambda-to-mock-PDS-in-Ref

Conversation

@amarauzoma
Copy link
Copy Markdown
Contributor

@amarauzoma amarauzoma commented Apr 17, 2026

  • Added a new Lambda function to simulate PDS responses for patient lookups.
  • Implemented rate limiting with configurable average and spike thresholds.
  • Updated existing MNS publisher to utilize the new PDS base URL environment variable.
  • Enhanced tests to cover scenarios with the mock PDS service, including rate limit handling.
  • Created performance testing scripts to validate MNS behavior under load with the mock PDS.
  • Updated infrastructure to route PDS calls in the ref environment to the mock endpoint.

amarauzoma and others added 2 commits April 17, 2026 12:51
- Added a new Lambda function to simulate PDS responses for patient lookups.
- Implemented rate limiting with configurable average and spike thresholds.
- Updated existing MNS publisher to utilize the new PDS base URL environment variable.
- Enhanced tests to cover scenarios with the mock PDS service, including rate limit handling.
- Created performance testing scripts to validate MNS behavior under load with the mock PDS.
- Updated infrastructure to route PDS calls in the ref environment to the mock endpoint.
- Streamlined the initialization of MockPdsService by consolidating parameters.
- Replaced multiple response methods with a unified error handling approach.
- Enhanced readability by reducing redundant code in the patient lookup logic.
- Updated tests to utilize helper functions for event creation and decision mocking.
@amarauzoma amarauzoma changed the title feat: Introduce Mock PDS Lambda for MNS integration testing VED-1235 Lambda-to-mock-PDS-in-Ref Apr 17, 2026
- Deleted the performance testing plan document for MNS integration with mocked PDS.
- This document outlined objectives, delivery approaches, and existing architecture for implementing a mocked PDS service.
…mock-PDS-in-Ref

Made-with: Cursor

# Conflicts:
#	tests/perf_tests/Makefile
- Removed the global service retrieval function and directly initialized the MockPdsService with a Redis client and rate limiter.
- Updated the lambda_handler to use the initialized service directly, improving performance and readability.
- Adjusted unit tests to accommodate the new service initialization approach, ensuring proper mocking and error handling.
… tests

- Updated sonar-project.properties to include coverage report for mock_pds.
- Added new unit tests for Mock PDS service to improve test coverage and handle edge cases.
- Introduced a new test file for rate limiting functionality in the Mock PDS service.
- Updated the quality-checks workflow to run tests for the new mock_pds service.
- Introduced a new Mock PDS Lambda function to simulate PDS responses.
- Created an ECR repository for the Mock PDS service with appropriate policies.
- Updated deployment workflows to include mock_pds in build flags and image overrides.
- Enhanced infrastructure configuration to support the new Mock PDS service, including image URI handling and environment variable setup.
Comment thread .github/workflows/deploy-backend.yml Outdated
Comment thread .github/workflows/deploy-backend.yml
Thomas-Boyle
Thomas-Boyle previously approved these changes Apr 22, 2026
Comment thread lambdas/mock_pds/pyproject.toml Outdated
[tool.poetry.dependencies]
python = "~3.11"
redis = "^6.1.0"
coverage = "^7.13.2"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be under dev dependencies - similar to other lambdas?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked and it seems that there no move is needed here as keeping coverage under tool.poetry.dependencies is consistent with how it is defined across the other lambdas in this repo.

count = var.mock_pds_enabled ? 1 : 0

function_name = aws_lambda_function.mock_pds_lambda[0].function_name
authorization_type = "NONE"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we doing authorization_type = NONE?
Does this mean this lambda is freely available from the internet?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, authorization_type = "NONE" makes the Function URL public. We used this to keep the mock PDS endpoint simple for lower-env integration/perf testing. It does not expose real PDS data, but you’re right that it is internet-reachable.

lambda_dir: ack_backend
- lambda_name: mock_pds
ecr_repository: imms-mock-pds-repo
lambda_dir: mock_pds
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean the mock_pds lambda will be created in all envs (including ref)? Dont we only need it for ref?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it won’t be created in all environments. mock_pds is in the deploy list, but Terraform only creates the Lambda when mock_pds_enabled=true.
That is only true in dev/ref, so it is only created in ref.


@staticmethod
def _get_method(event: dict) -> str:
return event.get("requestContext", {}).get("http", {}).get("method") or event.get("httpMethod") or "GET"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need the or GET - why are we protecting for it? wont we only use the method for get?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only allow GET, but this fallback is to support different API Gateway event formats. Some events provide the method at requestContext.http.method, others at httpMethod. We read either shape, then still validate that it is GET before serving a response.

return cls._response(status, {"code": int(status), "message": message})

@staticmethod
def _response(status_code: int, body: dict, content_type: str = "application/json") -> dict:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should status_code be HTTPStatus | int ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe there's any changes needed as HTTPStatus is an IntEnum and therefore already compatible with int in type checking/runtime.

Comment thread .github/workflows/quality-checks.yml Outdated
working-directory: lambdas/mock_pds
id: mock_pds
env:
PYTHONPATH: ${{ env.LAMBDA_PATH }}/mock_pds/src:${{ env.LAMBDA_PATH }}/mock_pds/tests
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this need to exclusively include tests? should it either be src or shared/src like other lambdas?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I’ve aligned it with the other lambdas and aligned it to be src

secret_manager_client=get_secrets_manager_client(),
environment=PDS_ENV,
environment = os.getenv("PDS_ENV", "int")
base_url = os.getenv("PDS_BASE_URL", "").strip() or None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: _pds_service is initialised on the first warm lambda invocation, from what I can tell. Therefore PDS_BASE_URL is read once at that point and put into PdsService object, subsequent invocations in the same warm container will not re-read it.

For the mock PDS, aws_lambda_function_url generates a new URL if the resource is ever destroyed and recreated (e.g. during a pr teardown), terraform will update the env var on the consuming Lambda, but warm containers will keep calling the old URL until they are recycled — causing silent PDS failures for an unpredictable window.

you could just ignore this keeping this in mind during your tests or move os.getenv("PDS_BASE_URL") inside [get_pds_service()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point about warm container caching and stale PDS_BASE_URL risk after URL recreation. I have implemented a config-aware cache refresh in get_pds_service. We still cache for warm invocation performance, but now reinitialize the PDS client automatically if PDS_BASE_URL or PDS_ENV changes, so we avoid stale URL behavior after mock PDS URL recreation.

Copy link
Copy Markdown
Contributor

@avshetty1980 avshetty1980 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, just a few nits.
I like the average + spike rate limiting with Redis atomicity per window

Comment thread lambdas/mock_pds/Dockerfile Fixed
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request infrastructure Pull requests that update terraform code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants