Add support for A-api-request-uuid query parameter #807
Conversation
|
There's a corresponding rain-api-core change for this: I think I will need to update the commit hashes in the dependency files in the PR once that is merged, to make sure it's getting pulled in here? |
|
@jeffersonwhite Please see the unit test and linter issues. |
|
I've updated to address the linter findings. For the unit tests here, they are failing because these changes depend on the changes I have in PR in rain-api-core. I'm not really sure what the process for making changes to both these repos should be, if I understand how things work I could:
I have run the tests locally to make sure that everything passes when all the changes are present. |
tests/test_app.py
Outdated
|
|
||
|
|
||
| def test_get_api_request_uuid(): | ||
| # Can't use the chalice test client here as it doesn't seem to understand the `{proxy+}` route |
There was a problem hiding this comment.
I don't think this comment applies because the function being tested is a helper function and not a request handler. You wouldn't be able to test it in isolation with the chalice test client anyway.
thin_egress_app/app.py
Outdated
| api_request_uuid = None | ||
| if query_params is not None: | ||
| api_request_uuid = query_params.get("A-api-request-uuid") | ||
| if api_request_uuid is not None: | ||
| log.info("A-api-request-uuid query param is " + api_request_uuid) | ||
| return api_request_uuid |
There was a problem hiding this comment.
api_request_uuid can just be local to the if statement.
| api_request_uuid = None | |
| if query_params is not None: | |
| api_request_uuid = query_params.get("A-api-request-uuid") | |
| if api_request_uuid is not None: | |
| log.info("A-api-request-uuid query param is " + api_request_uuid) | |
| return api_request_uuid | |
| if query_params is not None: | |
| api_request_uuid = query_params.get("A-api-request-uuid") | |
| if api_request_uuid is not None: | |
| log.info("A-api-request-uuid query param is " + api_request_uuid) | |
| return api_request_uuid | |
| return None |
The way I've done it in the past is to point the pinned dependency version to the latest commit hash from my feature branch. |
|
Looks like code cov might of made a change that broke our overage report. I am looking into it. I feel like it should not block your PR here as well. I will discuss with the team |
mattp0
force-pushed
the
api-request-uuid-support
branch
from
0f0286c to
2c097a1
Compare
|
@jeffersonwhite I got the codecov working. It looks like we just need to add a test or two to cover the cases when query params are |
…pport' into api-request-uuid-support
|
Thanks @mattp0 ! I've updated to cover those cases. |
|
@jeffersonwhite Thank you for the contribution! We will start the process of testing and will keep you updated once we have a release cut for TEA. |
When the A-api-request-uuid query parameter is included in the request, it will be added to the s3 presigned url so that it can be extracted as a metric when downloaded.