-
Notifications
You must be signed in to change notification settings - Fork 83
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
RFC300: Programmatic Toolkit #654
base: main
Are you sure you want to change the base?
Conversation
9f9403f
to
74205a4
Compare
|
||
#### Messages and Requests | ||
|
||
The toolkit emits *messages* and *requests* to structure interactions. |
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.
An example of a sequence of messages back and forth would be nice.
}); | ||
|
||
// Synth and store a cached CloudExecutable, this won't run synth again | ||
const cxCache = await cdk.synth(cx); |
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.
Is cxCache
the same type as cx
? Or implements the same interface?
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.
Yes see "Actions - synth". I don't want the example to be polluted by types. Would it be more clear to use let
here?
A detailed definition of what would constitute a breaking change is included in the readme above. | ||
|
||
To support integrations, we will need to publish a list of all messages. | ||
We will use static code analysis to automatically compile and publish this list without adding additional maintenance burden on to the team. |
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.
Any thoughts to how that would work, or TBD?
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 was thinking we could use the TS compiler to detect all calls to certain functions/methods and extract it that way.
No POC though.
bed1c2d
to
137b1af
Compare
137b1af
to
3f7072f
Compare
This is going to be a really useful capability that will enable many interesting and valuable use cases, including being able to bake CDK into an Electron app and compose, deploy and destroy CDK apps entirely in code. This will put the benefits of the CDK into the hands of a whole new audience - those who can't or won't install command line tools. I think it would be good to remove any interdependency between this and the consuming app around SDK, so consider if rather than passing an SdkProvider we could just pass a SDKv3 credentials provider. Also, one of the challenges I saw when trying to use the CDK programatically in the past is how it sends CFN outputs to STDOUT and STDERR - would be ideal if we wanted to consume the CFN stack creation events via CDK to have a way to tap into that feed. |
* **API Bar Raiser**: @rix0rrr | ||
|
||
The [Programmatic Toolkit] allows integrators to build lifecycle management solutions for CDK applications. | ||
It provides them with full control over all output and allows to interweave actions with custom steps. |
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.
and allows to interweave actions with custom steps.
Would be nice to include a quick example of this right off the bat.
#### Messages and Requests | ||
|
||
The toolkit emits *messages* and *requests* to structure interactions. | ||
A *request* is a *message* that requires the receiver to respond, otherwise the toolkit will continue with a default response. |
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.
Having a hard time imagining what the CLI would require a response for, and what default response would be. Can you show an example?
interface IoMessage<T> { | ||
time: string; | ||
level: 'error' | 'warn' | 'info' | 'debug'; | ||
action: 'synth' | 'list' | 'deploy' ... ; |
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.
Are these meant to represent the top level CLI command the user invoked? Or can they be sub actions like wait-for-stack-stabilization?
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.
Meant to be the invoked command, i.e. "what is the toolkit currently doing".
code
would be used for sub actions.
#### IoHost | ||
|
||
Message receivers need to implement the `IIoHost` interface. | ||
The `IoHost` is responsible for handling all logging and interactions: |
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.
The naming here is throwing me off. IoHost
seems to suggest it responds to I/O events like logging or network calls. But actually, it can respond to any event that we decide can be responded to...is that right? If so, is EventHost
a more appropriate name?
This implementation will do nothing for messages and requests that don't have a registered listener, i.e. the default response will be used as required. | ||
|
||
```ts | ||
const io = new IoEmitter(); |
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.
This implements IoHost
and dispatches to event listeners?
#### Readme | ||
|
||
The `AwsSdkProvider` defines how the toolkit interacts with AWS services. | ||
A default provider is available. | ||
You can provide a custom class implementing the `IAwsSdkProvider` interface. | ||
A standard `SdkProviderForCdk` is available that includes all features of the AWS CDK CLI [TODO: this might end up in a separate package]. |
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 don't follow. Is this part of the previous section? Also it doesn't sound like an alternate solution, but rather a possible future extension?
|
||
#### Event stream | ||
|
||
> Streams are non blocking by design. |
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.
> Streams are non blocking by design. | |
> Disregarded. Streams are non blocking by design. |
It also implies refactoring of existing interfaces to match the new API design. | ||
Both of these things are prone to human errors. | ||
|
||
Mitigation for this is test coverage and separating out PRs that are purely moving code, from actual changes. |
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.
We probably want to coordinate rolling this out only after we address the coverage gaps we detected.
|
||
No. | ||
|
||
### What alternative solutions did you consider? |
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.
What about the option of not providing an event API at all, and instead just focus on the objects the CLI returns, and make sure they contain all the information necessary for customers to take actions?
That is:
// no hooks available on the `cdk` instance
const deployment = cdk.deploy(...);
// take the output and do something
const stacks = deployment.listStacks();
....
I'm not really advocating for this approach, but I want to understand exactly what use-cases this solution doesn't address.
| FR10 | Events | | ||
| | In extension to logging focused events, the CDK should emit events for API operations including progress and state transitions (e.g. for deployment: synth complete, assets built, assets published, Hotswap/CFN deployment started ...). If available, these events should include additional data like the arn of the stack or a progress percentage. An integrator must be able to execute blocking code on these events. This will allow them to e.g. schedule pre-build scripts before synth starts or to update a service registry after deployment completed. | | ||
| FR11 | Event hooks | |
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.
Can you map these requirements to specific use-cases? that is, specific actions we know customers want to take inside events.
This will help us understand if the use case can be addressed in some other way.
Co-authored-by: Eli Polonsky <[email protected]>
const deployment = await cdk.deploy(cxCache, { stacks: ['MyTestStack'] }); | ||
|
||
// Returns a list of ARNs of the deployment | ||
return deployment.listResources().filter(r => r.arn); |
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.
Does this method also allow access to resource attributes like AWS::Lambda::Url
's FunctionUrl
? I could see this being very helpful for automated testing.
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.
Unfortunately I don't think that's easily possible as CFN stack APIs don't include this information. You'd need to use a Stack output or make the request to the service API manually. But at least that should be much easier.
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.
Stack outputs would work fine, but how would accessing those values work under this proposal?
For example, deployment.listOutputs()
to get the outputs? Or perhaps deployment.describe()
to wrap calls to DescribeStacks for the descriptions of all the deployed stacks?
const deployment = await cdk.deploy(cxCache, { stacks: ['MyTestStack'] }); | ||
|
||
// Returns a list of ARNs of the deployment | ||
return deployment.listResources().filter(r => r.arn); |
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.
Is there the possibility of filtering resources here by node path? This might make it easier to locate specific resources for a system under test.
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.
This is a great suggestion! Might not be in the first version, but should be easy to add.
This is a request for comments about Programmatic Toolkit. See #300 for
additional details.
Rendered Version
APIs are signed off by @rix0rrr.By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache-2.0 license