-
Notifications
You must be signed in to change notification settings - Fork 4k
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(region-info): Model region-specific information #1839
Conversation
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.
I think the configuration strategy here is going to make it hard to configure in a way that's not going to make us tear our hairs out, or get it wrong in subtle ways.
I'd propose we switch to a more generic rules engine in which we can do things like (hypothetical syntax):
built_after_2014(_) = true
built_after_2014(us-east-1) = false
built_after_2014(eu-west-1) = false
// ...
s3_format(region) = if built_after_2014(region) then 's3-website.${region}.${domain}' else 's3-website-${region}.${domain}';
I would also like us to think about escape hatching -- how will users override the values we define (if we get them wrong) or missing values (if it's a region for which we don't have information yet). I'd prefer for users to supply some data files rather than them having to write code.
I'm suggesting rules engine because I imagine it'll be easier to extend with data points and rules after the fact, by users. Some well-crafted JS should be able to fulfil the same role. Not sure which one I prefer |
Here are reasons I can come up with against JS:
I'm trying to find an article that I've read that says something along the lines of "use TABLES not RULES", but I can't find it. |
The system I implemented allows a user to register their own region (can write code to source this from files somewhere - I don't need this right now so I don't code this right now). You can also replace an existing region with your own if we messed it up (and it's simple enough to "override" the existing region if you wanted - so you only change what you mean to... But I guess fine about the rule engine (or a table?) assuming I find it legible and not too complicated... |
Here are some reservations I still have:
|
@RomainMuller any updates on this? |
@eladb Was on-call last week so made no progress, but I will be making a brand new version of this accounting for all the conversations that were had on the subject. Expect a force-push there... in some not-so-distant future. |
No worries. Just checking in :-) |
Allows accessing region-specific information such as the name of the partition the region is in, service principals used in-region, and other static information that is useful when building infrastructure.
d6695ad
to
7bb5cc1
Compare
Force-pushed a new approach on there, early stages (not using the data yet). Looking to receive early feedback. |
packages/@aws-cdk/region-info/build-tools/generate-static-data.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/region-info/build-tools/static-data-facts/index.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/region-info/build-tools/static-data-facts/index.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/region-info/build-tools/static-data-facts/promised-region-info.ts
Outdated
Show resolved
Hide resolved
If a package's `package.json` file contains a `jest` entry (used for configuring the behavior of `jest`), assert the package uses `jest` instead of `nodeunit` as it's test harness (and fail if there are any `nodeunit` tests - based on file name). This is going to make it easier to migrate away from `nodeunit`.
Information such as:
Fixes #1282
Pull Request Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.