Skip to content
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

[request] Allow overriding of request configuration on fluent request builders #537

Closed
2 tasks
jszwedko opened this issue May 18, 2022 · 6 comments
Closed
2 tasks
Assignees
Labels
feature-request A feature should be added or improved.

Comments

@jszwedko
Copy link
Contributor

Describe the feature

Hi all!

We have a feature request on vector to allow setting the x-amzn-logs-format HTTP header to json/emf when publishing events to CloudWatch Logs via PutLogEvents to send embedded metrics.

I found an example of doing it with the Java SDK:

PutLogEventsRequest putLogEventsRequest = PutLogEventsRequest.builder()
                        .overrideConfiguration(builder ->
                                // provide the log-format header of json/emf
                                builder.headers(Collections.singletonMap("x-amzn-logs-format",  Collections.singletonList("json/emf"))))
                        .logEvents(Collections.singletonList(inputLogEvent))
                        .logGroupName(logGroupName)
                        .logStreamName(logStreamName)
                        .sequenceToken(sequenceToken)
                        .build();

But I don't see a similar sort of method for overriding request headers for the Rust SDK. Am I missing something? Or would this be a feature request?

Use Case

Overriding request headers on a per API call-basis.

Proposed Solution

Add a method to all fluent builders that allows overriding request configuration including headers.

Here is what the Java SDK seems to support overriding:

https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/core/RequestOverrideConfiguration.html

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

A note for the community

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue, please leave a comment
@jszwedko jszwedko added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels May 18, 2022
@Velfi Velfi removed the needs-triage This issue or PR still needs to be triaged. label May 18, 2022
@Velfi
Copy link
Contributor

Velfi commented May 18, 2022

Thanks for submitting this, I have an idea of how to support this and I'll run it by the team to see what they think.

@Velfi
Copy link
Contributor

Velfi commented May 18, 2022

@jszwedko The way to accomplish this task today is to:

  1. Have users construct the operation directly (i.e. not using the fluent builder)
  2. Call Operation::into_parts to get access to the inner request
  3. Add the header to that request
  4. Reconstruct the operation by calling Operation::from_parts
  5. Send the operation off using a smithy client

That's obviously not a very friendly experience. The much nicer solution would require adding a new API to allow converting a fluent builder into some type Sendable<O> (this type doesn't exist and doesn't necessarily need to be named Sendable) that has methods for modifying the inner operation and updating operations with an Operation::augment method that allowed users to pass a callback that would modify the inner parts of an op.

@rcoh imagines the UX looking like this:

let sendable = dynamodb.list_tables().limit(5).<workshop work here>();
sendable.map_operation(|op| op.augment(...).send().await;

We'd need an RFC proposal covering all this before moving forward with a PR. I'm currently absorbed in checksum work relating to S3 but I may be able to look at this afterwards. I can't give any time estimate or guarantee though.

If you're interested in writing and submitting an RFC for this, we can coach you on the process. Let me know if that's how you'd like to proceed.

@jszwedko
Copy link
Contributor Author

Thanks for the response @Velfi ! I missed that the request could be split into parts and reconstructed. We can give that a shot. Otherwise that UI you mentioned looks decent to me to be able to compose the operations similar to what the Java SDK allows for.

@Velfi
Copy link
Contributor

Velfi commented Aug 18, 2022

I have a PR implementing John's RFC that should make things much nicer.

@jdisanti
Copy link
Contributor

It is now possible with the latest SDK to add this header. Take a look at the notes for release-2022-09-21 for an example.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
Archived in project
Development

No branches or pull requests

3 participants