Skip to content

Expose the metadata key on client interfaces #2327

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 2 commits into from
Apr 2, 2021

Conversation

abrooksv
Copy link
Contributor

@abrooksv abrooksv commented Mar 11, 2021

Description

Add a new static field on the client interfaces with metadata ID (aka API endpoint prefix)

Motivation and Context

#2301

Testing

public interface ApiGatewayClient extends SdkClient {
    String SERVICE_NAME = "apigateway";

    /**
     * Value for looking up the service's metadata from the
     * {@link software.amazon.awssdk.regions.ServiceMetadataProvider}.
     */
    String SERVICE_METADATA_ID = "apigateway";
public interface ApiGatewayAsyncClient extends SdkClient {
    String SERVICE_NAME = "apigateway";

    /**
     * Value for looking up the service's metadata from the
     * {@link software.amazon.awssdk.regions.ServiceMetadataProvider}.
     */
    String SERVICE_METADATA_ID = "apigateway";
public interface EcrAsyncClient extends SdkClient {
    String SERVICE_NAME = "ecr";

    /**
     * Value for looking up the service's metadata from the
     * {@link software.amazon.awssdk.regions.ServiceMetadataProvider}.
     */
    String SERVICE_METADATA_ID = "api.ecr";

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed
  • A short description of the change has been added to the CHANGELOG
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@abrooksv abrooksv force-pushed the apiPrefixField branch 3 times, most recently from ac9e0c0 to 92fc8d3 Compare March 11, 2021 20:48
"globalEndpoint": "json-service.amazonaws.com",
"protocol": "rest-json",
"serviceAbbreviation": "Json Service",
"serviceFullName": "Some Service That Uses Json Protocol",
"serviceId":"Json Service",
"serviceId": "Json Service",
"signingName": "json-service",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added so it tests when singing name != endpoint prefix

@abrooksv
Copy link
Contributor Author

abrooksv commented Mar 11, 2021

Digging into this more it seems there is a

    static ServiceMetadata serviceMetadata() {
        return ServiceMetadata.of(SERVICE_METADATA_ID);
    }

on the sync interface (it is missing from the async interface).

This is basically what I needed the ID for in the first place, but on order to make what I need work (dynamic service info in the IDEs), I basically need to find a way to update MetadataLoader with an updated endpoints.json data that the toolkits team vends from CloudFront today. Note: The toolkits are onboarded to the SDK release platform, that is how we get this data today.

/**
 * Internal class for determing where to load region and service
 * metadata from. Currently only generated region metadata is supported.
 */
@SdkInternalApi
final class MetadataLoader {

Is there any thoughts on how to make the Currently only generated region metadata is supported. a bit less true of a statement 😉 To be clear, I am not advocating for dropping the codegen, but a way to load newer data into the MetadataLoader using the SDKs endpoints metadata format.

The endpoints.json parser currently lives in codegen-lite, so it would have to be added to new module that lives below codegen-lite and regions.


The other more work for us approach would be to roll our own ServiceMetadata system, but I would like to leverage the endpoints.json loader given how complicated the resolve logic is. In that case, I would still need the ID to be exposed.

@Quanzzzz
Copy link
Contributor

Quanzzzz commented Apr 1, 2021

Sorry for delay. This pr should be good to go once we get another review from the team, as this is a change to the public apis. About the extra feature request mentioned in the comment, we can implement a new utillity method to read the service metadata directly from the json file so you can get "dynamic service info". We've created a new feature request task in our backlog, will implement it once it's prioritized.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 2, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@Quanzzzz Quanzzzz merged commit 0370309 into aws:master Apr 2, 2021
@zoewangg
Copy link
Contributor

zoewangg commented Apr 6, 2021

@all-contributors please add @abrooksv for code

@allcontributors
Copy link
Contributor

@zoewangg

I've put up a pull request to add @abrooksv! 🎉

aws-sdk-java-automation added a commit that referenced this pull request Jan 13, 2023
…09ffbb424

Pull request: release <- staging/e77e56f0-b289-43b0-8a65-92c09ffbb424
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.

3 participants