-
Notifications
You must be signed in to change notification settings - Fork 82
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.
|
||
##### SdkProvider | ||
|
||
[Alternative: Make this `CredentialsProvider` or `SdkConfigProvider` instead] |
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 an opportunity to try to redesign the SDK abstraction, to allow for less traumatic version updates.
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.
It is. I feel we might want to treat this separately then and not include this right away, but wait until we had a retro on SDK version updates.
}); | ||
|
||
// 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?
```ts | ||
const cdk = new Toolkit({ | ||
ioHost: new MyIoHost(), | ||
awsSdkProvider: new MyAwsSdkProvider(), |
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 it's probably fine to do something more simple here, and only provide configuration for the SDK.
It's still possible to do what we always do: if we feel we need more control, add an optional sdkProvider?:
later and use runtime checks to make it mutually exclusive with configuration properties.
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.
bdc19f8
to
15ee78f
Compare
Co-authored-by: Rico Hermans <[email protected]>
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