Skip to content

[C++] SDKs to track 3 feature Ids for S3 functionality #3506

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 1 commit into from
Aug 7, 2025
Merged

Conversation

kai-ion
Copy link
Contributor

@kai-ion kai-ion commented Aug 1, 2025

Issue #, if available:

Description of changes:

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@@ -1504,4 +1504,4 @@ namespace Aws
}
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed new line in file without changing file

@@ -52,6 +53,10 @@ XmlOutcome AWSXMLClient::MakeRequest(const Aws::AmazonWebServiceRequest& request
const char* signerRegionOverride /* = nullptr */,
const char* signerServiceNameOverride /* = nullptr */) const
{
// Add S3Express user agent feature if backend is S3Express
if (endpoint.GetAttributes() && endpoint.GetAttributes()->backend == "S3Express") {
const_cast<Aws::AmazonWebServiceRequest&>(request).AddUserAgentFeature(Aws::Client::UserAgentFeature::S3_EXPRESS_BUCKET);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we adding the cast const_cast<Aws::AmazonWebServiceRequest&> ? its not needed?

EXPECT_FALSE(response.IsSuccess());

// Verify S3_EXPRESS_BUCKET feature (value 6) was added
const auto& features = request.GetUserAgentFeatures();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isnt actually testing it the other user agent test captures the request that was seen in the http client -- this is not using the captured mock request, it is jsut using the request that you submitted to the s3 client

@kai-ion kai-ion force-pushed the mato branch 2 times, most recently from 581d37f to f5a8524 Compare August 6, 2025 20:06
- added S3_express_bucket tracking

updated to only include tracking for s3_express

updated to only include tracking for s3_express

updated to only include tracking for s3_express

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

s3 unit test

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent
@kai-ion kai-ion merged commit 995bc6e into main Aug 7, 2025
5 of 6 checks passed
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.

2 participants