Allow separate service ID and alias #211
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note to the reviewers: I'm happy to discuss changing the template variable |
a-hilaly
approved these changes
Sep 29, 2021
a-hilaly
approved these changes
Sep 30, 2021
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: A-Hilaly, RedbackThomson, vijtrip2 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jaypipes
reviewed
Oct 4, 2021
| @@ -34,6 +31,9 @@ type MetaVars struct { | |||
| // for custom resources, e.g. "sns.services.k8s.aws" or | |||
| // "sfn.services.k8s.aws" | |||
| APIGroup string | |||
| // AWSSDKModelServiceID is the exact string that appears in the AWS service API's | |||
| // api-2.json descriptor file under `metadata.serviceId` | |||
| AWSSDKModelServiceID string | |||
There was a problem hiding this comment.
this probably could have just been SDKModelServiceID to match the SDKAPIInterfaceTypeName field below.
RedbackThomson
pushed a commit
to RedbackThomson/ack-code-generator
that referenced
this pull request
Oct 7, 2021
This reverts commit 15c7f5c.
ack-bot
pushed a commit
that referenced
this pull request
Oct 7, 2021
acornett21
pushed a commit
to acornett21/ack-code-generator
that referenced
this pull request
Oct 12, 2021
Description of changes: Reverts aws-controllers-k8s#214 and aws-controllers-k8s#211 in favour of using the `generator.yaml` file to store the service model name By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
ack-bot
pushed a commit
that referenced
this pull request
Oct 13, 2021
Issue #, if available: aws-controllers-k8s/community#994 Description of changes: This pull request replaces all templates and variables with a defined nomenclature. The current code-generator uses service name terms interchangeably, which eventually lead to the linked issue. While PR #211 solved the logical issue of overriding the service model name, in some cases, it did not do a clear job of clarifying what names exist for any service and when they are applicable. There are 3 different names that are used to programmatically reference a service. Here are the terms I am using in this PR to refer to them, and their given purpose: - Service ID - The `metadata.serviceID` field from the model `api-2.json` file - Service model name - The path to the model `api-2.json` file - Service package name - The AWS SDK Go package path - Used by the ACK controller's for their name Some examples on when these differ: - Step Functions uses `sfn` for its ID and alias, but `states` for its model name - Elastic Load Balancing v2 uses `elbv2` for its alias, but `elasticloadbalancingv2` for its ID and model name There are also 2 names defined in the controller metadata file that are not used in any parts of the code, but are used in documentation: - Service name - The full name of the service as defined by the product's documentation page (eg. `Amazon Elastic Cloud Compute`) - Service short name - An informal, shortened name of the service (eg. `EC2`) By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes aws-controllers-k8s/community#994
Description of changes:
This pull request supports including an optional
--model-namecommand line argument for anyack-generategenerator verb. The generator will use this argument to override the service name when looking up the API files inaws-sdk-go/models/apis.Currently we reference the
metadata.serviceIdfield from theapi-2.jsonfile as theServiceIDCleanvariable.ServiceIDCleanis used in all code generator templates as the import path foraws-sdk-goand when referencing the controller name (eg.{{ .ServiceIDClean}}-controller). This pull request will redirectServiceIDCleanto use the service alias provided when callingack-generate, instead. Therefore, all subsequent ACK repositories should be named according to the AWS SDK Go package name, rather than the API file's definition ofserviceId.ServiceIDhas been removed from the template variables, instead replaced byAWSSDKModelServiceIDif it is needed.ServiceIDCleannow refers to the name of the AWS SDK Go package name.For services that need to use the
--model-namecommand line argument, such asopensearchserviceandelbv2, developers should use theACK_GENERATE_MODEL_NAMEenvironment variable when callingmake build-controller. Changes to the CI/CD system will need to be made to accommodate this customisation.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.