feat(naming): adds a ShardingNamingConvention that can inspect the detail component of a Name #28
Conversation
| "foo" || "foo" || null || [] | ||
| "x1foo-bar-baz" || "-bar-baz" || [1: s(1, "foo")] || [] | ||
| "x1foo-x2bar-x1baz-bazinga" || "-bazinga" || [1: s(1, "baz"), 2: s(2, "bar")] || [dupe(1, "foo", "baz")] | ||
| "x2bar-x1foo-blah-x3baz" || "-blah-x3baz" || [1: s(1, "foo"), 2: s(2, "bar")] || [remains("-blah-x3baz", 3, "baz")] |
There was a problem hiding this comment.
Ideally we could precisely reproduce the overall resource name if we know the component parts. The support for arbitrary ordering and de-duping means that it isn't possible to do that reliably.
There was a problem hiding this comment.
as we discussed on chat - I think the fact that a NamingConvention is used to inspects a name after it has been processed into an app/stack/detail may put us in a stronger spot that we previously were (we always only use those three components (and push sequence for a deploy) to compose the final name)
|
How is the NamingConvention expected to get used with Names? Would Names be used to extract the detail and then the user would use the naming convention class directly? What do you think about moving the old labelled vars stuff into a naming convention? The main change to the current behavior is that now these labelled vars would be considered as part of the detail string. This came up recently because we found an example in use where Frigga can generate names using AutoScalingGroupNameBuilder that it cannot parse back out reliably. Example: def 'should be able to parse generated name'() {
when:
AutoScalingGroupNameBuilder cluster = new AutoScalingGroupNameBuilder()
cluster.withAppName("app")
.withStack("stack")
.withDetail("c0usa")
Names name = Names.parseName(cluster.buildGroupName())
then:
"app-stack-c0usa" == cluster.buildGroupName()
"app" == name.app
"stack" == name.stack
"detail" == name.detail // fails because detail matches a labelled var
}The labelled var pattern does not appear to be in use at Netflix. We could make it a naming convention and have it only get applied and used in Names if one of the labelled var accessors is called. Thoughts? |
|
yeah I do like the idea of extracting all the existing labelledVars into a NamingConvention (or potentially just deleting them all..) |
|
Deleting them all is fine with me, but I haven't done any due diligence to see if it would break something. |
prevent capturing anything that is -[alpha]0 as a labeled variable and explicitly scopes to the already implemented variables
…etail string An optional naming convention that can be applied against a freeFormDetails string so as not to inflict the opinion on all frigga users
…ention Breaking Accessor methods still exist on Names and calling one will parse labeled variables out of the cluster name, preserving the existing values as they would be returned. Labeled variables are not dropped prior to considering the app/stack/detail components of a name, so where previously a cluster called foo-c0banana would have a stack of null and a country of banana, now will have a stack of c0banana and a country of banana.
cfieber
force-pushed
the
sharding_support
branch
from
1002615
to
57d08e7
Compare
| sequence = Integer.parseInt(sequenceString); | ||
| } catch (NumberFormatException e) { | ||
| // This is fine. | ||
| protected Names(String name) { |
There was a problem hiding this comment.
reworked the constructor here to make the fields final
| } else if (!zone.equals(other.zone)) | ||
| return false; | ||
| return true; | ||
| public boolean equals(Object o) { |
There was a problem hiding this comment.
can safely just consider group for equals and hashCode, since that is the original value passed in the constructor, everything else is determined off that value at construction, and group is only non-null if the supplied name was legitimately parseable
| @@ -341,7 +341,7 @@ class NamesSpec extends Specification { | |||
| 'cass-nccpintegration-random-junk-c0northamerica-d0prod-h0gamesystems-p0vizio-r027-u0nccp-w0A-z0useast1a' == names.cluster | |||
| 'cass' == names.app | |||
| 'nccpintegration' == names.stack | |||
| 'random-junk' == names.detail | |||
| 'random-junk-c0northamerica-d0prod-h0gamesystems-p0vizio-r027-u0nccp-w0A-z0useast1a' == names.detail | |||
There was a problem hiding this comment.
changes in this test highlight the breaking change behavior from refactoring out the labeled variables into a NamingConvention
A NamingConvention describes a way of extracting extra attributes from a name without
requiring that all consumers adopt that convention.