-
Notifications
You must be signed in to change notification settings - Fork 909
Use partitions.json to generate Region class #6120
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"type": "feature", | ||
"category": "AWS SDK for Java v2", | ||
"contributor": "", | ||
"description": "Updated Region class generation to use Partitions.json instead of the Endpoints.json and removed the hardcoded global regions." | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,19 +59,24 @@ public class RegionGenerationMojo extends AbstractMojo { | |
"${basedir}/src/main/resources/software/amazon/awssdk/regions/internal/region/endpoints.json") | ||
private File endpoints; | ||
|
||
@Parameter(property = "partitionsjson", defaultValue = | ||
"${basedir}/src/main/resources/software/amazon/awssdk/regions/internal/region/partitions.json.resource") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use the same file in codegen? Otherwise, we'd need to update our release system to keep it updated and if it's not possible, I'd prefer to keep the file in codegen-lite instead of core since we don't want anyone to use it or load it at runtime. |
||
private File partitionsjson; | ||
|
||
@Override | ||
public void execute() throws MojoExecutionException { | ||
Path baseSourcesDirectory = Paths.get(outputDirectory).resolve("generated-sources").resolve("sdk"); | ||
Path testsDirectory = Paths.get(outputDirectory).resolve("generated-test-sources").resolve("sdk-tests"); | ||
|
||
Partitions partitions = RegionMetadataLoader.build(endpoints); | ||
Partitions regionpartitions = RegionMetadataLoader.build(partitionsjson); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto here - is there a reason not to name this Alternative naming thought - rename There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, that's a good idea. I'll rename the partitions above to legacyPartitions and the new one to just partitions. |
||
|
||
generatePartitionMetadataClass(baseSourcesDirectory, partitions); | ||
generateRegionClass(baseSourcesDirectory, partitions); | ||
generateRegionClass(baseSourcesDirectory, regionpartitions); | ||
generateServiceMetadata(baseSourcesDirectory, partitions); | ||
generateRegions(baseSourcesDirectory, partitions); | ||
generateRegions(baseSourcesDirectory, regionpartitions); | ||
generatePartitionProvider(baseSourcesDirectory, partitions); | ||
generateRegionProvider(baseSourcesDirectory, partitions); | ||
generateRegionProvider(baseSourcesDirectory, regionpartitions); | ||
generateServiceProvider(baseSourcesDirectory, partitions); | ||
generateEndpointTags(baseSourcesDirectory, partitions); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,17 +62,61 @@ public final class Partition { | |
*/ | ||
private Endpoint defaults; | ||
|
||
/** | ||
* The partition id. | ||
*/ | ||
private String id; | ||
|
||
/** | ||
* Configuration outputs for the partition. | ||
*/ | ||
private PartitionOutputs outputs; | ||
|
||
public Partition() { | ||
} | ||
|
||
public Partition(@JsonProperty(value = "partition") String partition, | ||
@JsonProperty(value = "regions") Map<String, PartitionRegion> | ||
regions, | ||
@JsonProperty(value = "services") Map<String, | ||
Service> services) { | ||
Service> services, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like its a little weird to squish both the legacy endponts.json partitions and the new partitions metadata (partitions.json) model together - How much of a refactor would be required to add a new PartitionsMetadata model that models only the new partitions.json? I think that would make the code in RegionGenerationMojo more clear as well (since its not clear to me which of the generation functions require which version of partitions). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, creating a separate PartitionsMetadata model would be more clear. However, the main issue is that some generation functions like generateServiceProvider and generateEndpointTags still require data from endpoints.json, as services and endpoints aren't present in partitions.json. So for now, using partitions.json only for region generation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that makes sense - I'm not suggesting that we completly replace the existing Partition model since its still required. I'm just suggesting that we create a new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, got it. I'll make that change. |
||
@JsonProperty(value = "id") String id, | ||
@JsonProperty(value = "outputs") PartitionOutputs outputs, | ||
@JsonProperty(value = "regionRegex") String regionRegex) { | ||
this.partition = Validate.paramNotNull(partition, "Partition"); | ||
this.regions = regions; | ||
this.services = services; | ||
this.id = id; | ||
this.outputs = outputs; | ||
this.regionRegex = regionRegex; | ||
} | ||
|
||
/** | ||
* Returns the partition ID. | ||
*/ | ||
public String getId() { | ||
return id; | ||
} | ||
|
||
/** | ||
* Sets the partition id. | ||
*/ | ||
public void setId(String id) { | ||
this.id = id; | ||
} | ||
|
||
/** | ||
* Returns the configuration outputs for the partition. | ||
*/ | ||
public PartitionOutputs getOutputs() { | ||
return outputs; | ||
} | ||
|
||
/** | ||
* Sets the configuration outputs for the partition. | ||
*/ | ||
public void setOutputs(PartitionOutputs outputs) { | ||
this.outputs = outputs; | ||
} | ||
|
||
/** | ||
|
@@ -177,6 +221,82 @@ private boolean matchesRegionRegex(String region) { | |
return p.matcher(region).matches(); | ||
} | ||
|
||
public static class PartitionOutputs { | ||
private String dnsSuffix; | ||
private String dualStackDnsSuffix; | ||
private String implicitGlobalRegion; | ||
private String name; | ||
private boolean supportsDualStack; | ||
private boolean supportsFIPS; | ||
|
||
@JsonProperty("dnsSuffix") | ||
public String getDnsSuffix() { | ||
return dnsSuffix; | ||
} | ||
|
||
public void setDnsSuffix(String dnsSuffix) { | ||
this.dnsSuffix = dnsSuffix; | ||
} | ||
|
||
@JsonProperty("dualStackDnsSuffix") | ||
public String getDualStackDnsSuffix() { | ||
return dualStackDnsSuffix; | ||
} | ||
|
||
public void setDualStackDnsSuffix(String dualStackDnsSuffix) { | ||
this.dualStackDnsSuffix = dualStackDnsSuffix; | ||
} | ||
|
||
@JsonProperty("implicitGlobalRegion") | ||
public String getImplicitGlobalRegion() { | ||
return implicitGlobalRegion; | ||
} | ||
|
||
public void setImplicitGlobalRegion(String implicitGlobalRegion) { | ||
this.implicitGlobalRegion = implicitGlobalRegion; | ||
} | ||
|
||
@JsonProperty("name") | ||
public String getName() { | ||
return name; | ||
} | ||
|
||
public void setName(String name) { | ||
this.name = name; | ||
} | ||
|
||
@JsonProperty("supportsDualStack") | ||
public boolean getSupportsDualStack() { | ||
return supportsDualStack; | ||
} | ||
|
||
public void setSupportsDualStack(boolean supportsDualStack) { | ||
this.supportsDualStack = supportsDualStack; | ||
} | ||
|
||
@JsonProperty("supportsFIPS") | ||
public boolean getSupportsFIPS() { | ||
return supportsFIPS; | ||
} | ||
|
||
public void setSupportsFIPS(boolean supportsFIPS) { | ||
this.supportsFIPS = supportsFIPS; | ||
} | ||
} | ||
|
||
public static class Region { | ||
private String description; | ||
|
||
@JsonProperty("description") | ||
public String getDescription() { | ||
return description; | ||
} | ||
|
||
public void setDescription(String description) { | ||
this.description = description; | ||
} | ||
} | ||
|
||
@Deprecated | ||
private boolean hasServiceEndpoint(String endpoint) { | ||
for (Service s : services.values()) { | ||
|
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.
Is there a reason the
j
is lower case? (ie, why notpartitionsJson
?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.
Sorry, my bad. I'll update it.