[CFN-27] Working towards a more extensible DBInstance create flow.... #574
Conversation
| final Optional<DBCluster> maybeDbCluster = DBInstancePredicates.isDBClusterMember(model) | ||
| ? Optional.of(fetchDBCluster(rdsProxyClient, model)) | ||
| : Optional.empty(); |
There was a problem hiding this comment.
Can we not use optionals, it adds more cognitive load for the reader, can we instead use method overloads for when there is a dbcluster vs when there isn't?
From Brian Goetz himself: https://stackoverflow.com/questions/26327957/should-java-8-getters-return-optional-type/26328555#26328555
There was a problem hiding this comment.
We can not-use Optionals here. But let's avoid taking the java authors word as gospel. What they intended Optionals for says nothing about what they are good for. What they are good for is avoiding null. The cognitive load of an optional is really just the cognitive load of a nullable reference, but it's in your face so you/we are more likely to handle it correctly.
|
What is CFN-27? |
…atic methods and error rules have been pulled out of the base handler class; The instance create factory is now constructed by factory according to the model. [CFN-27] Continuing to seperate db-instance create flows. This pulls out the create from snapshot logic at the cost of some more copy pasta to be addressed in the next commit. [CFN-27] The 'safeAddTags' method is still c/p but the core change in pattern has finished resulting in a trim create step in the create handler (line 103). ...
npxcomplete
force-pushed
the
polymorphic-create-flow
branch
from
77e00a3
to
9727a77
Compare
Incomplete, for early feedback from the other core maintainers. We (AWS RDS) have a number of upcoming features that stand to benefit from presenting an cleaner way to extend the create paths. The core team have discussed what this might look like and this is a first pass attempt to separate shape the code to that vision.
Description of changes: Static methods and error rules have been pulled out of the base handler class; The instance create factory is now constructed by factory according to the model.
I plan to follow up with a second revision but wanted to stop here as the decomposition of the BaseHandlerStd, while simple, has become somewhat large. The "FreshInstance" factory has copied a few methods that need to be reconciled.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.