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

feat(core): add AccountIdEndpointMode config option #6036

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

siddsriv
Copy link
Contributor

@siddsriv siddsriv commented Apr 30, 2024

Issue

Internal JS-4633
Part 3

Description

Adds support for AccountIdEndpointMode config option. (client config, env, config file)

Testing

  • Unit tests
  • Generate the client config option

Checklist

  • If you wrote E2E tests, are they resilient to concurrent I/O?
  • If adding new public functions, did you add the @public tag and enable doc generation on the package?

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

@siddsriv siddsriv requested a review from a team as a code owner April 30, 2024 05:01
@siddsriv siddsriv marked this pull request as draft April 30, 2024 05:01
@siddsriv siddsriv changed the title feat(core): add AccountIdEndpointMode config feat(core): add AccountIdEndpointMode config option Apr 30, 2024
if (isAwsService(settings, model) && target.equals(LanguageTarget.NODE)) {
writer.addDependency(AwsDependency.AWS_SDK_CORE);
writer.addImport("emitWarningIfUnsupportedVersion", "awsCheckVersion", AwsDependency.AWS_SDK_CORE);
writer.write("awsCheckVersion(process.version);");
Copy link
Contributor Author

@siddsriv siddsriv May 14, 2024

Choose a reason for hiding this comment

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

want to confirm importing /core here is okay, since a chunk of this work will be placed in /core itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

outdated question^

switch (target) {
case BROWSER:
runtimeConfigs.put("accountIdEndpointMode", writer -> {
writer.addDependency(TypeScriptDependency.CONFIG_RESOLVER);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this dependency should be for core instead, if we were to place defaults and resolver in core instead of the config-resolver package.

* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

fix copyright statements to the short version

import java.util.Map;
import java.util.function.Consumer;
import java.util.logging.Logger;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

put these on the top, remove spacing between import statements

if (isAwsService(settings, model)) {
ServiceShape service = settings.getService(model);
EndpointRuleSetTrait endpointRuleSetTrait = service.getTrait(EndpointRuleSetTrait.class)
.orElseThrow(() -> new RuntimeException("service missing EndpointRuleSetTrait"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

possibly don't throw here

RuleSetParameterFinder ruleSetParameterFinder = new RuleSetParameterFinder(service);
if (ruleSetParameterFinder.getBuiltInParams().containsKey("AccountIdEndpointMode")) {
writer.writeDocs("Defines if the AWS AccountId will be used for endpoint routing.")
.write("accountIdEndpointMode?: AccountIdEndpointMode | __Provider<AccountIdEndpointMode>;\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

need to import this type with the writer

}

@Override
public void prepareCustomizations(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this entire function can be omitted

writer.addDependency(TypeScriptDependency.CONFIG_RESOLVER);
writer.addImport("DEFAULT_ACCOUNT_ID_ENDPOINT_MODE", "DEFAULT_ACCOUNT_ID_ENDPOINT_MODE",
TypeScriptDependency.CONFIG_RESOLVER);
writer.write("(() => Promise.resolve(DEFAULT_ACCOUNT_ID_ENDPOINT_MODE))");
Copy link
Contributor

Choose a reason for hiding this comment

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

async () => val

Map<String, Consumer<TypeScriptWriter>> runtimeConfigs = new HashMap<>();
if (isAwsService(settings, model)) {
EndpointRuleSetTrait endpointRuleSetTrait = service.getTrait(EndpointRuleSetTrait.class)
.orElseThrow(() -> new RuntimeException("service missing EndpointRuleSetTrait"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dont throw here either

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.

None yet

2 participants