-
Notifications
You must be signed in to change notification settings - Fork 1
[GPCAPIM-255] Controller module #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[GPCAPIM-255] Controller module #59
Conversation
…vironment." This reverts commit f1a3fad.
…le getting in to their own testable methods.
…ed to recognise it in the app.
…such a restrictive path reduces the ability to place test files within module directories.
…SonarQube complaining.
…herrypick Signed-off-by: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com>
https://github.com/NHSDigital/clinical-data-gateway-api into feature/GPCAPIM-255_controller_integration_cherrypick
- Change dependency groups for several packages to include "main" - Ensure requests version is consistent across dependencies
davidhamill1-nhs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass
| ], | ||
| ) | ||
|
|
||
| def set_response_from_flaskresponse(self, flask_response: FlaskResponse) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what of the aim of this. But that may be because of my unnecessary handler class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding was that we were passing the request body and status received from the provider back through unchanged? That being the case there needs to be a mechanism for setting _response_body to the value returned as the body from the controller. This is that.
Does that make sense or have I got the wrong end of the stick somewhere?
| :returns: A :class:`~gateway_api.common.common.FlaskResponse` representing the | ||
| outcome. | ||
| """ | ||
| auth_token = self.get_auth_token() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not at all job for now, more a note for future work. We will be making multiple requests to NHS APIs through this Flask app. We should be clever about how we handle our auth journey, and grab an auth token, use that one for as long as it lasts and then get another one. This could be done by having the client, just before the PDs/SDS call, use some common functionality to get a cached token if possible, otherwise get a new token. In this way we are call the /auth endpoint as little as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 @DWolfsNHS I can't remember, did we have a Confluence page for recording this sort of observation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be worth having a call to discuss how to test the controller.
|
|
✅ Trivy gate: no Critical/High vulnerabilities. Trivy Image Scan SummaryImage: 900119715266.dkr.ecr.eu-west-2.amazonaws.com/whoami:feature-gpcapim-255-controller-integration-cherrypick
✅ No vulnerabilities found. |
|
Deployment Complete
|


Description
Creates the controller class that orchestrates calls to the other gateway components and to the GP provider
Context
This module is needed so that we have something that actually calls all the components that we need to call and manages their responses.
Supersedes #49. See #49 for discussion/comments specifically on the controller. This PR incorporates integrating the controller with API entry point, PDS and GP provider client.
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.