Use partitions.json to generate Region class #6120
Conversation
|
| @@ -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 = | |||
There was a problem hiding this comment.
Is there a reason the j is lower case? (ie, why not partitionsJson?
There was a problem hiding this comment.
Sorry, my bad. I'll update it.
| 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.
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.
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.
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 PartitionsMetadata model and use it in the places where we can. That way we don't have Partition objects that have missing/incomplete data (eg, in the current code, regionpartitions will have empty service maps, since partitions.json doesn't have that information).
There was a problem hiding this comment.
Yeah, got it. I'll make that change.
| @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.
Ditto here - is there a reason not to name this regionPartitions?
Alternative naming thought - rename partitions above to legacyPartitions and the new one to just partitions?
There was a problem hiding this comment.
Sure, that's a good idea. I'll rename the partitions above to legacyPartitions and the new one to just partitions.
| @@ -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.
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.
Updated Region class generation to use Partitions.json instead of the Endpoints.json and removed the hardcoded global regions.
Motivation and Context
Modifications
Testing
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License