[GPDAPIM-258] SDS access module #79
Conversation
|
✅ Trivy gate: no Critical/High vulnerabilities. Trivy Image Scan SummaryImage: 900119715266.dkr.ecr.eu-west-2.amazonaws.com/whoami:feature-gpcapim-258-sds-access-module
Findings (top 50)
|
|
✅ Trivy gate: no Critical/High issues. Trivy IaC (Terraform) Summary
Findings (top 50)
|
|
|
Deployment Complete
|
| "urn:nhs:names:services:gpconnect:structured" | ||
| ":fhir:operation:gpc.getstructuredrecord-1" | ||
| from gateway_api.common.common import ( | ||
| ACCESS_RECORD_STRUCTURED_INTERACTION_ID as ARS_INTERACTION_ID, |
There was a problem hiding this comment.
Given this is now nicely defined in common, is it better to just rename in this module than alias?
| ENDPOINT: Literal["Endpoint"] = "Endpoint" | ||
|
|
||
| # Default service interaction ID for GP Connect | ||
| DEFAULT_SERVICE_INTERACTION_ID = ACCESS_RECORD_STRUCTURED_INTERACTION_ID |
There was a problem hiding this comment.
Now that this is imported from common, do we need this intermediate variable?
| self.base_url = base_url.rstrip("/") | ||
| self.timeout = timeout |
There was a problem hiding this comment.
If this is defined locally, or passed from the controller can we be confident that it is already well formated?
| self.stub = SdsFhirApiStub() | ||
|
|
||
| # Use stub for now - use environment variable once we have one | ||
| # TODO: Put this back to using the environment variable | ||
| # if os.environ.get("STUB_SDS", None): | ||
| self.get_method: GetCallable = self.stub.get | ||
| # else: | ||
| # self.get_method: GetCallable = requests.get |
There was a problem hiding this comment.
No functional change, but I think this is also part of the temporary code to point to the stub, so covered by the same comment.
| self.stub = SdsFhirApiStub() | |
| # Use stub for now - use environment variable once we have one | |
| # TODO: Put this back to using the environment variable | |
| # if os.environ.get("STUB_SDS", None): | |
| self.get_method: GetCallable = self.stub.get | |
| # else: | |
| # self.get_method: GetCallable = requests.get | |
| # Use stub for now - use environment variable once we have one | |
| # TODO: Put this back to using the environment variable | |
| # if os.environ.get("STUB_SDS", None): | |
| self.stub = SdsFhirApiStub() | |
| self.get_method: GetCallable = self.stub.get | |
| # else: | |
| # self.get_method: GetCallable = requests.get |
| asid = self._extract_identifier(device, self.ASID_SYSTEM) | ||
| party_key = self._extract_identifier(device, self.PARTYKEY_SYSTEM) | ||
|
|
||
| # Step 2: Get Endpoint to obtain endpoint URL |
There was a problem hiding this comment.
This infers that party_key is only included in the response, and that is being used to determine if the we should also make a request to Endpoint. Is that correct?
| timeout=timeout or self.timeout, | ||
| ) | ||
|
|
||
| # TODO: Post-steel-thread we probably want a raise_for_status() here |
There was a problem hiding this comment.
raise_for_status() may be the proportionate middle ground in steel thread. Let's disucss...
| # --------------- internal helpers for result extraction ----------------- | ||
|
|
||
| @staticmethod | ||
| def _extract_first_entry( |
There was a problem hiding this comment.
I think handling the case where we get more than one entry is more than we needed in Steel Thread, but no harm in handling it here
|
|
||
| def test_sds_client_get_org_details_success( | ||
| stub: SdsFhirApiStub, # noqa: ARG001 | ||
| mock_requests_get: dict[str, Any], # noqa: ARG001 |
There was a problem hiding this comment.
| mock_requests_get: dict[str, Any], # noqa: ARG001 | |
| mock_requests_get: dict[str, Any], # noqa: ARG001 retained throughout as this will be required again once temporary code tested class to direct all `get` to stub is removed. |
2 participants
Description
Creates the SDS access module and integrates it with the controller.
Context
CDG requires the ability to obtain organisation ASIDs and endpoints from SDS. This provides that capability.
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.