Skip to content

add X-Request-Id in UMAPI GET calls #96

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

Merged
merged 3 commits into from
Feb 15, 2022

Conversation

shasibhusanJena
Copy link
Contributor

Summary

  • set X-Request-Id in UMAPI GET calls with uuid.uuid4()

Testing Steps

  • when we make GET calls to the UMAPI, we should set the X-Request-Id header to a Universally Unique ID

Fixes #95

@Luci2015
Copy link
Contributor

the uuid.uuid4() in headers={"X-Request-Id": uuid.uuid4()} is not a string and won't do what you intend.
also I see that there is a self.session.headers declared already - why don't you make use of that one to get your header in the API calls?

also, in make_call I see there is a delete branch, but AFAIK UAMPI uses only POST and GET. maybe wanna get that checked as well.

@adorton-adobe
Copy link
Contributor

The UUID object ought to be cast to str downstream but we should probably explicitly convert it when setting the header. @shasibhusanJena can you take care of that?

Regarding the other items:

Headers passed to a session call are appended to session.headers. I tested this when I added sync signals and can confirm it works this way.

I believe reason delete exists because the API used to accept DELETE to manage invitations. See https://adobe-apiplatform.github.io/umapi-documentation/en/api/ManageInvites.html

We can look at removing it, but that is outside the scope of this PR.

@Luci2015
Copy link
Contributor

Oh, that one is deprecated years ago..

@adorton-adobe
Copy link
Contributor

I'm aware, but umapi-client was first written before the functionality was deprecated. The client doesn't appear to provide any kinf of interface to those calls, so we can probably remove that case. But it's still out of scope for this PR, so please open a new issue if you want us to look into it.

@shasibhusanJena
Copy link
Contributor Author

sure @adorton-adobe . i thing this is going to be only change
will change headers={"X-Request-Id": uuid.uuid4()} to headers={"X-Request-Id": str(uuid.uuid4())}
other then that will check if anything else needed

@adorton-adobe adorton-adobe merged commit b5c16af into adobe-apiplatform:v2 Feb 15, 2022
@adorton-adobe adorton-adobe mentioned this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set the X-Request-Id header to a Universally Unique ID (UUID)
3 participants