Add CDK support #53
Add CDK support #53
Conversation
This commit implements full support for the Basti construct. All features are now working (based on a limited test). Testing still remains, but requires more investigation to set up.
|
@BohdanPetryshyn If you could provide some feedback on this while I continue to work on setting up tests, that would be much appreciated. |
|
@bobveringa, I will be able to review the PR tomorrow morning. Thank you! |
Add unit tests to validate CDK implementation
|
A couple of things that still need to be figured out. Setting the author correctly so that it can be published to PyPI. We also need to determine who is the official maintainer of this package. And maybe we have to set up GitHub actions for basti in general? |
100%, it was sometimes hard for me to publish even a single Basti CLI package manually
I thought I will be maintaining everything unless you are willing to help with this 🤗 |
|
Setting up actions might be slightly more complicated with the monorepo structure, but I doubt basti is the only package using that approach, so there must be some solution. Yeah, it makes sense to have you as maintainer for everything. In that case, you might need to set up a PyPI account, I'll investigate this and make sure this can be done swiftly. |
|
Thank you @bobveringa! I have a very busy day today so I'll only be able to reply to your comments in the evening or tomorrow morning |
Clarify what basti ID means to avoid confusion.
Add support for github actions
|
That's ok, I'm, not in a hurry. I also noticed that the package-lock.json should probably be removed from the basti package. https://docs.npmjs.com/cli/v9/using-npm/workspaces |
Add support for github actions
Add support for github actions
Looks like there is a package by AWS for this |
Add support for github actions
There was a problem hiding this comment.
Thank you for such a comprehensive approach to contribution (docs and the GH action) 👍
What about scheduling a call for a quick "demo"? It would be awesome to see the construct in action. Also, I will be able to better understand the building a publishing process, ask some more questions, and provide more feedback. Let's agree on a call on Linkedin.
I found some typos in the docs. But in general, I plan to improve consistency and apply other style-related minor changes in docs later on my own. This will be much more efficient than the review flow.
You're right. Could you please, do this in your PR? |
Apply review feedback for the Basti CDK feature.
|
@bobveringa, we have only two unresolved conversations here. We're good to go after resolving these 🎉 |
Apply review feedback for the Basti CDK feature.
Remove tagging support
Stop the propagation of tags to the volume now that Tags are managed differently.
Proposed Changes
This PR adds initial support for CDK to Basti. It is currently still a draft for gathering feedback.
Related Issue
[Closes #48]
Changes
The
.eslintrc.jsonfile was modified because WebStorm as going crazy about line endings and this made it silent.It contains a
README.mdbut only as template.Users have the option of explicitly setting the ID of the basti instance. If the user does not provide one, it generates one instead based on the sha1 hash of the construct ID. This is "random enough" for the usecase.
The basti instance itself is also configurable with a custom machine image and instance type. This could be expanded more in the future.
The subnet is selected automatically to whatever Public subnet cloudformation things it can use. However, users do have the option to pass a specific
SubnetSelectionobject so this can be modified if needed.The
BastiInstancealso has an additional method calledgrantBastiCliConnectthis function takesIGrantableas a parameter and allows developers to grant roles (and IAM users) permissions to connect to the basti instance using the basti CLI. In the future, the construct might support other methods that would allow other services to connect. But for now, this will suffice.