feat: formation #550
feat: formation #550
Conversation
|
Not turbo formation? |
.projenrc.ts
Outdated
| @@ -0,0 +1,391 @@ | |||
| import * as fs from "fs"; | |||
There was a problem hiding this comment.
hmm, must have come from the merge? deleted
| typeof kind === "number" ? getNodeKindName(kind) : kind | ||
| ) | ||
| .join(", ")}` | ||
| .join(", ")}, found: ${item}` |
There was a problem hiding this comment.
Will this print an object usefully?
There was a problem hiding this comment.
Not always, but it did when I was testing. Will stringify.
| busName: addr.node.addr, | ||
| busName: bus.resource.eventBusName, |
There was a problem hiding this comment.
Oops lol. How did that ever work
There was a problem hiding this comment.
It did work, but the bus was a singleton.
| })); | ||
| } | ||
|
|
||
| async function nodeCfnDeploy( |
| "node-cfn": "bin/node-cfn.js", | ||
| "node-cfn.js": "bin/node-cfn.js" |
There was a problem hiding this comment.
likely, just copied them over
| circularReferences.push(logicalId); | ||
| break; |
There was a problem hiding this comment.
Should you continue and find all?
There was a problem hiding this comment.
This was your code, I rewrote it in the next commit to use a pure DFS and support memoized paths.
There was a problem hiding this comment.
I can comment on my own code too. It's all being committed 😅
There was a problem hiding this comment.
I re-wrote all of this anyways, lol. It was re-doing a lot of work.
| } | ||
|
|
||
| /** | ||
| * TODO: support optional snapshot. |
There was a problem hiding this comment.
Some resources support snapshots on delete. My thought was the interface would have an optional snapshot callback that can be implemented by a provider when they want to support snapshots.
Additionally, they should be able to force a snapshot on delete for cases like RDS.
| "AWS::CDK::Metadata": new NoOpProvider(), | ||
| "AWS::Lambda::EventSourceMapping": (props) => | ||
| new EventSourceMappingProvider(props), | ||
| [DEFAULT_RESOURCE_PROVIDER_KEY]: (props) => new CloudControlProvider(props), |
There was a problem hiding this comment.
Would it a be better to have this logic handled explicitly by the caller? Strange to see it as a magic key
There was a problem hiding this comment.
these are defaults, the user can pass in any map of providers or merge into the defaults.
What interface were you looking for?
There was a problem hiding this comment.
I would expect this map to only have resource types. What resource is the default? Is there logic to specially handle that key?
There was a problem hiding this comment.
Default is the provider we try when there is no explicit key for the current resource in the map. It seemed easier to define the default providers this way, at least until a more complex fallback system was created.
Other options:
- a map of key to providers and a parameter for "defaultProvider"
- a pipeline approach where a single provider is given which can have providers nested within.
- a map of key to providers then an array of fallback providers.
- a single handler given by the consumer which returns the provider given a key and some config
| constructor(props: ResourceProviderProps) { | ||
| this.controlClient = new control.CloudControlClient(props.sdkConfig); | ||
| } | ||
| retry: ResourceProviderRetryConfig = { canRetry: true }; |
There was a problem hiding this comment.
Set this in the constructor? Make it read only?
There was a problem hiding this comment.
Moved it to the top and made it readonly. No reason to set in the constructor if it is a constant.
There was a problem hiding this comment.
Why even have it if it's a constant? I was just thinking for consistency. It's jarring code
There was a problem hiding this comment.
Its configuration per provider. Not all providers can idempotently be retried on all operations.
My though was to have a nested package ( |
If we have multiple packages, they should all share the same branding. We need to agree on the branding. I think turbo formation was winning last time we chatted. We can bring it up again with the team |
V0.1 of
@functionless/formation