-
Notifications
You must be signed in to change notification settings - Fork 1.1k
add user agent business metric for account id endpoints #3381
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
Conversation
0177284
to
a2db6cf
Compare
92af8c2
to
d808ae8
Compare
} | ||
|
||
ClientConfiguration::ClientConfiguration() | ||
{ | ||
this->disableIMDS = false; | ||
setLegacyClientConfigurationParameters(*this); | ||
setConfigFromEnvOrProfile(*this);; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting fact -- this meant functionally that the disableIMDSv1
configuration did not work if a user set a region in configuration
Aws::String accountId; | ||
|
||
/** | ||
* The AccountId Endpoint Mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comparison is done by
strcmp(mapping.first, accountIdMode.c_str()) == 0; });
i.e. not case insensitive.
In an ideal world, I'd suggest to use an enum here.
On practice, please just add a comment with possible allowed values and the requirements of the lowercase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this is a better user experience, however this would be backwards breaking because on main because of how codegen is currently generating it.
#include <aws/core/Aws.h>
#include <aws/dynamodb/DynamoDBClient.h>
using namespace Aws;
using namespace Aws::DynamoDB;
auto main() -> int {
SDKOptions options;
InitAPI(options);
{
DynamoDBClientConfiguration config;
config.accountIdEndpointMode = "preferred";
DynamoDBClient client{config};
}
ShutdownAPI(options);
return 0;
}
In an ideal world/On practice
yep yep, will add comment regarding
f5568c5
to
61b1cbe
Compare
61b1cbe
to
5f9d664
Compare
Description of changes:
Adds business metrics for the account id endpoint feature. Moves account id configuraiton from client specific paramters to general parameters as we should have initially.
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.