-
Notifications
You must be signed in to change notification settings - Fork 48
initial implementation #2
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
initial implementation #2
Conversation
|
@tobywf have added a proposed structure for the plugin, would you mind to give it a quick look and let me know if you think it seems like a good way to proceed. There is no generated sources like in the java plugin, there is just a python module that implements a handler_wrapper that will be the entrypoint and import the customer impllemented handlers in |
|
Apologies, been on vacation - will take a look shortly! |
python/rpdk/python/cfn_resource/cfn_resource/handler_wrapper.py
Outdated
Show resolved
Hide resolved
python/rpdk/python/cfn_resource/cfn_resource/handler_wrapper.py
Outdated
Show resolved
Hide resolved
|
runtime handler implemented with some rudimentary tests. It's a pretty big pr at this point, I'll put some comments in on areas that I'd like some extra attention on. @tobywf @latrokles would appreciate a brutal (but constructive) review |
rjlohan
left a comment
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've left a bunch of comments throughout. So that you're not playing endless churn, I'd look to merge this initial cut in, and come back around with smaller PRs to adjust to catch-up to the Java plugin PRs.
Check my inline comments, and perhaps file issues in this repo on those so you can track?
tobywf
left a comment
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 can merge this for now. #4 should be addresses ASAP, and will be a requirement before distributing the plugin to a wider audience. The linting will also catch issues like using key in dict.keys() instead of key in dict
…operation-status-skip-stack-hook Skip stack level hook for stack if prior stack level change set hook …
This provides an initial implementation loosely based on the java plugin. In order to be idiomatic for python developers no attempt will be made to keep the resulting resource template similar to the java plugin. Tests will be mimicked as closely as possible to try to provide some parity.
TODO:
Will add the following in an upcoming pr:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.