Add support for A-api-request-uuid query parameter

asfadmin / thin-egress-app

API Gateway & Lambda AWS Thin Egress App

Other

17 stars 17 forks source link

Add support for A-api-request-uuid query parameter #807

Closed jeffersonwhite closed 5 months ago

jeffersonwhite commented 6 months ago

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.

jeffersonwhite commented 6 months ago

There's a corresponding rain-api-core change for this: https://github.com/asfadmin/rain-api-core/pull/292/files

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?

mattp0 commented 6 months ago

@jeffersonwhite Please see the unit test and linter issues.

jeffersonwhite commented 6 months ago

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:

Update this PR to temporarily point the rain-api-core dependency to the branch of my forked repo so it passes the tests
Wait until the rain-api-core PR is merged, and then update the dependency here to point to the new commit

I have run the tests locally to make sure that everything passes when all the changes are present.

reweeden commented 6 months ago

* Update this PR to temporarily point the rain-api-core dependency to the branch of my forked repo so it passes the tests

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.

mattp0 commented 5 months ago

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 commented 5 months ago

@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 None You can see the report here https://app.codecov.io/gh/asfadmin/thin-egress-app/commit/2c097a10259b1fa61f73766dd03acfff1190892e

jeffersonwhite commented 5 months ago

Thanks @mattp0 ! I've updated to cover those cases.

mattp0 commented 5 months ago

@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.