open-constructs / aws-cdk-library

Community-Driven CDK Construct Library
Apache License 2.0
44 stars 6 forks source link

Covering functionality that is already covered elsewhere #13

Closed dbartholomae closed 1 week ago

dbartholomae commented 1 month ago

Recently, the question came up whether we should https://github.com/open-constructs/aws-cdk-library/pull/8#issuecomment-2041600083.

The goal of this issue is to prepare discussion around this and form it into an ADR. My first impression from checking in Slack is that adopting community constructs is one of the purposes of this library, with the goal to reduce risks for others adopting them as the trust in the open constructs foundation will transfer to the constructs adopted by this library.

I would also assume that we consciously do not adopt any constructs without the consent of their maintainers, even if licenses would allow us to.

I'll leave this issue open for a week so that we can collect consenting and dissenting opinions. For consent, a simple thumbs-up is enough, for dissent please write a comment so I know what you would do differently.

Then I'll write an ADR based on this which is also open to debate (though hopefully no big directional surprises anymore).

mrgrain commented 1 month ago

I think this is a fine goal.

It does however open a sub-discussion if and how we want to resolve "conflicts" between multiple solutions on offer. In this specific case PR #8 is competing with my package mrgrain/cdk-esbuild and probably others as well.

For obvious reason I am of the opinion that my package is the best thing ever (hehe ;) ) and I'd be happy to donate it. However I'm also not sure I can spend much time on moving it over here, plus there are some questions that are still a bit unclear for me in regards to how maintenance works and about certain design choices I made. But that's maybe a separate discussion.

I don't really have an answer for this, nor a strong opinion which of the two solutions (or both) should be accepted in this case.

dbartholomae commented 1 month ago

Good question! My personal preference would be that we research alternatives and reach out to other maintainers, so that we can discuss together the advantages and disadvantages of different solutions. This might then even lead to a best-of-breed solution that started with one community approach but also incorporates learnings from others who worked on the same topic. Apart of the actual solution, I think that talking into account the ability for the original maintainer to support maintaining could also make sense from a practical perspective.

I'm not sure yet how to actually achieve this without potentially creating a hostile atmosphere, though. We should not end up pitting maintainers against each other to "win the honour of being part of this project" or something like that. My goal would be to create an atmosphere of collaboration to get the best out of all solutions, but there can always be trade-off calls with differing opinions.

mrgrain commented 1 month ago

This sounds like a balanced approach to me. And it's okay to be somewhat aspirational. How things play out in practice is a different question anyway.

michanto commented 1 month ago

Of course I think that PR #8 is easier to maintain (small amount of code, no heroics to get esbuild), and useful (code is in a file, not a string) but I'd be happy to incorporate parts of other solutions, or demure to a preferred solution if one is chosen. I'd also be happy with an approach of starting simple and adding features later if people need them and are willing to maintain them.

Whatever we do, we should do it with a mind towards making this library something that people will see as official enough to include in their CDK projects without reservation - a trusted source.

mbonig commented 1 month ago

The initial goal of this library is not to create a general library with all sorts of "random" community-created constructs, but to augment the development of L2 constructs that has been turned away by the AWS team specifically. While I think the long-term goal is that we do have a nice curated place for general third-party constructs that are outside of just the normal scope of L2 constructs, I'd prefer we focus on getting some L2s created by the community that the AWS team will not accept rather than incorporating in constructs that the AWS team would never accept anyway (and that the third party community is already building).

dbartholomae commented 1 month ago

Good point! I feel like this is a bit of an orthogonal discussion - whether something is new or a missing L2, in either case there might be preexisting community constructs out there.

But we should also document the point you made: what kind of constructs do we want to have in the library anyways? Especially since there will always be corner cases. E.g. a construct to create code assets from TypeScript feels like a glaring omission from the existing L2s for CloudWatch Canaries (and some others) to me, and while I would ideally expect such a fundamental primitive to be part of aws-cdk itself, I would see this as a good alternative home.

I'll open a separate issue for this tomorrow.

mrgrain commented 1 month ago

I have the relatively strong opinion that anything related to bundling and building is NOT an L2. The L2 contract for Assets (here via Code) is effectively to provide an S3 location. And uploading a file to S3 as an Asset is easily done with existing stuff.

I could probably be convinced though, that converting Construct props to a JSON like Asset is in scope for L2s.

dbartholomae commented 4 weeks ago

I've split out the discussion about which constructs should be in the library into #14.

dbartholomae commented 2 weeks ago

I've opened a small PR #20 based on what we discussed so far.

hoegertn commented 1 week ago

This can be closed, correct?

dbartholomae commented 1 week ago

Yes!