From f3908bebd588282df6fb42f58c02bab83399d082 Mon Sep 17 00:00:00 2001 From: Alain Krok Date: Thu, 13 Feb 2025 11:48:59 -0600 Subject: [PATCH] chore(PR): doc update and code generation issue (#958) * chore(doc): update documentation --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .github/workflows/code-generation.yml | 2 +- .projenrc.ts | 4 ++ CONTRIBUTING.md | 25 +++----- DEPRECATION_PROCESS.md | 40 +++++++++++++ DESIGN_GUIDELINES.md | 84 +++++++++++++++++++-------- DEVELOPER_GUIDE.md | 9 +-- README.md | 5 ++ projenrc/github-workflows.ts | 2 +- 8 files changed, 124 insertions(+), 47 deletions(-) create mode 100644 DEPRECATION_PROCESS.md diff --git a/.github/workflows/code-generation.yml b/.github/workflows/code-generation.yml index 5a295309..165eeb01 100644 --- a/.github/workflows/code-generation.yml +++ b/.github/workflows/code-generation.yml @@ -70,7 +70,7 @@ jobs: git config user.email "github-actions@github.com" - name: Create Pull Request id: create-pr - uses: peter-evans/create-pull-request@38e0b6e68b4c852a5500a94740f0e535e0d7ba54 + uses: peter-evans/create-pull-request@67ccf781d68cd99b580ae25a5c18a1cc84ffff1f with: token: ${{ secrets.PROJEN_GITHUB_TOKEN }} commit-message: |- diff --git a/.projenrc.ts b/.projenrc.ts index 5235e217..b6098956 100644 --- a/.projenrc.ts +++ b/.projenrc.ts @@ -245,6 +245,10 @@ project.github?.actions.set( 'peter-evans/create-pull-request@v6', 'peter-evans/create-pull-request@b1ddad2c994a25fbc81a28b3ec0e368bb2021c50', ); +project.github?.actions.set( + 'peter-evans/create-pull-request@v7.0.6', + 'peter-evans/create-pull-request@67ccf781d68cd99b580ae25a5c18a1cc84ffff1f', +); project.github?.actions.set( 'aws-actions/configure-aws-credentials@v4.0.2', 'aws-actions/configure-aws-credentials@e3dd6a429d7300a6a4c196c26e071d42e0343502', diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 99073128..c9055a7b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -49,35 +49,28 @@ If you are proposing a new AWS Generative AI CDK Construct, the best way to do t Once the design is finalized, you can re-purpose this PR for the implementation, or open a new PR to that end. Good AWS Generative AI CDK Constructs have the following characteristics: - 1) Multi-service: The goal of AWS Generative AI CDK Constructs is to weave multiple services together in a well-architected way. - 2) Configurable Business Logic: AWS Generative AI CDK Constructs should be applicable to all businesses and workloads as much as possible so that they are easily reusable. - 3) Reusable across multiple use-cases: We would rather have a small library of constructs that are wildly popular with customers rather than a huge library of constructs that customers find irrelevant. - 4) Well Architected: AWS Generative AI CDK Constructs should be secure, reliable, scalable, and cost efficient. + 1) Configurable Business Logic: AWS Generative AI CDK Constructs should be applicable to all businesses and workloads as much as possible so that they are easily reusable. + 2) Reusable across multiple use-cases: We would rather have a small library of constructs that are wildly popular with customers rather than a huge library of constructs that customers find irrelevant. + 3) Well Architected: AWS Generative AI CDK Constructs should be secure, reliable, scalable, and cost efficient. + 4) They simplify complex service configurations while maintaining security and cost controls by default. + 5) They provide clear, predictable interfaces focused on business outcomes rather than technical implementation details. + 6) They integrate seamlessly with existing AWS services and follow established deployment patterns and best practices. ### Step 3: Work your Magic Now it's time to work your magic. Here are some guidelines: * Coding style (abbreviated): - * In general, follow the style of the code around you - * 2 space indentation - * 120 characters wide - * ATX style headings in markdown (e.g. `## H2 heading`) + * In general, follow the style of the code around you. The linter will run on every PR and modify files. * Every change requires a unit test * If you change APIs, make sure to update the module's README file * Try to maintain a single feature/bugfix per pull request. It's okay to introduce a little bit of housekeeping changes along the way, but try to avoid conflating multiple features. Eventually all these are going to go into a single commit, so you can use that to frame your scope. -* If your change introduces a new construct, take a look at the our - [example construct]() for an explanation of the L3 patterns we use. +* If your change introduces a new construct, take a look at our [example construct](./src/cdk-lib/bedrock/) for an overview of existing code. Feel free to start your contribution by copy&pasting files from that project, and then edit and rename them as appropriate - it might be easier to get started that way. -* To ensure CDKv2 compatibility of all the Generative AI Constructs, please ensure the code meets the following guidelines: - * Import statement for `Construct` is standalone, for example, `import { Construct } from '@aws-cdk/core';` instead of `import { Construct, App, Aws } from '@aws-cdk/core';` - * Check to make sure the usage of `Construct` in the code is also standalone, for example, `export class IotToSqs extends Construct` insted of `export class IotToSqs extends cdk.Construct` - * Core classes are imported from `@aws-cdk/core` only, for example, `import { Duration } from "@aws-cdk/core;` instead of `import { Duration } from "@aws-cdk/core/lib/duration";` - * DO NOT USE deprecated APIs, it will not build in CDKv2, for example, using `statistic?` attribute of [@aws-cdk/aws-cloudwatch.Alarm](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_cloudwatch.Alarm.html) Construct Props will fail to build in CDKv2 #### Integration Tests @@ -141,7 +134,7 @@ BREAKING CHANGE: Description of what broke and how to achieve this behavior now - The Build workflow - controlled by the buildWorkflow field. On a ‘pull_request’ or ‘workflow_dispatch’ the library will be built and checked for anti-tamper (ensure no manual changes to generated files). - The Release workflow - controlled by the releaseWorkflow field. On a push to main (overridden at props.defaultReleaseBranch) the library is built, anti-tampered, version bumped with a commit, pushed back to git, and then published to the configured artifact repositories (e.g. npm, pypi). -By default, for every commit to the default (main) branch, a new version is released (trunk-based development). This includes the following steps: +Every commit to the default (main) branch marked as feat or fix will trigger a new version release (trunk-based development). This includes the following steps: - Compile, lint and test the code. - Use JSII to produce library artifacts for all target languages. diff --git a/DEPRECATION_PROCESS.md b/DEPRECATION_PROCESS.md new file mode 100644 index 00000000..394afb85 --- /dev/null +++ b/DEPRECATION_PROCESS.md @@ -0,0 +1,40 @@ +# Deprecation process + +## Path 1: Graduation to AWS CDK Core + +In some cases, if an existing construct meets criteria and graduates to the AWS CDK core library, the following deprecation process is applied: + +- Mark the construct as deprecated in code using annotation. For instance, in the construct constructor, add: +``` +Annotations.of(scope).addWarningV2('@cdklabs/generative-ai-cdk-constructs:NAME_OF_THE_CONSTRUCT.deprecation', + 'This construct is deprecated and will not receive further support besides critical bug fixes. It will be removed on DATE. Please follow the documentation to migrate.'); +``` +- Update the construct's documentation with notice of graduation and link to the new CDK core construct +- Expected end-of-support date (typically 6 months) +- Create a migration guide that includes: + - Step-by-step migration instructions + - Code examples for both old and new implementations + - Breaking changes and new features + - Maintain critical bug fixes only until end-of-support date + +## Path 2: Low Adoption Deprecation + +For constructs with consistently low adoption: + +- The core team reviews usage metrics over 3-month period +- If usage remains below threshold, mark construct as deprecated in code. For instance, in the construct constructor: +``` +Annotations.of(scope).addWarningV2('@cdklabs/generative-ai-cdk-constructs:NAME_OF_THE_CONSTRUCT.deprecation', + 'This construct is deprecated and will not receive further support besides critical bug fixes. It will be removed on DATE. Please refer to the documentation for additional information.'); +``` +- Document rationale for deprecation +- Add sunset period: usually 3 month period +- Suggest alternative approaches or constructs + +After sunset period, remove the code from the library and release a new version. + +## For both paths: + +- Deprecation will be announced in CHANGELOG, in the specific construct README and in the construct code +- A GitHub issue will be created for tracking +- Notification via release notes and discussions \ No newline at end of file diff --git a/DESIGN_GUIDELINES.md b/DESIGN_GUIDELINES.md index 1d2f2c32..a74c66e7 100644 --- a/DESIGN_GUIDELINES.md +++ b/DESIGN_GUIDELINES.md @@ -10,33 +10,78 @@ This document defines the ways in which Constructs are consistent. When proposin ## Overall Guidelines -**Use CDK definitions to define a service or resource** +**Avoid custom resources** -The construct should not create new classes or interfaces to describe services or resources. Although the new class may seem simpler now, as new capabilities are added to the construct the new class will acquire new properties – the ultimate result would be something equivalent to the CDK definition, but not compatible. The CDK definitions are well thought out and interact predictably with other CDK constructs, use them. If you want a client the ability to specify a few attributes of a ConstructProps without specifying every required value, then make the type of that attribute ConstructProps | any. This pattern exists several places in the Generative AI Constructs library. +Even though this construct library is experimental, we will challenge contributions containing custom resources (CRs). CRs have been historically used in this library, and they have caused several issues: +- They are difficult to maintain and debug +- They introduce additional failure points and complexity +- They often require external dependencies (Lambda functions) +- They can lead to inconsistent behavior across different AWS regions +- They may break when AWS makes changes to underlying services -Another practice that this rule prohibits is putting specific attributes of sub resources in your Generative AI Constructs Props object. For instance - if your VPC needs an Internet Gateway, then the client should send VPC Props that create the Internet Gateway, don't create a property at in your Construct Props object of InternetGateway: true. +While custom resources might be justified in specific scenarios (e.g., interacting with third-party services or implementing unique functionality), we will review these cases individually. Instead, prefer using native AWS constructs and services whenever possible, even if it means accepting some limitations in functionality. -**The client should have the option (but not requirement) to provide any props used within the construct** +**Use Core CDK design guidelines** -When you supply a CDK defined props object to any CDK constructor within your construct (L1 or L2), your construct should be able to generate all the default values required to create that L1 or L2 construct. Alternatively, the client should be able to override any or all of those default props values to completely customize the construct. +The core CDK library provides a [design guidelines document](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md). The purpose of this document is to provide guidelines for designing the APIs in the AWS Construct Library in order to ensure a consistent and integrated experience across the entire AWS surface area. -There are exceptions to this rule. The Lambda Function L2 construct, for instance, cannot specify a default value for the Lambda code – the client must provide this. Also, at times the workings of the construct require specific props values to work. In this situation, those values will override user provided values. In general, the props values sent to a CDK constructor are generated like this: +This is even more important if you design a construct which should eventually graduate to the core CDK library. For instance, if you are building a L2 construct for a service which doesn't have an official L2 construct yet, following those guidelines is crucial to make the construct compliant and streamline the review process. -``` -let actualProps = overrideProps(defaultProps, userProvidedProps); -actualProps = overrideProps(actualProps, constructRequiredProps) -``` +**Keep L3 Constructs focused and manageable** -Construct required props should be minimized and listed in the README.md file for the Construct. -There is a possibility that the client could specify some props values that are incompatible with default props values. That is the responsibility of the client – if they choose to not use the default implementation then they take responsibility for the configuration they specify. +L3 constructs should be designed with a "do one thing and do it well" philosophy. While it's tempting to create large, feature-rich constructs that handle multiple use cases, experience has shown that such constructs often suffer from low adoption rates and impose significant cognitive load on users. Those constructs require significant maintenance efforts, and provide limited flexibility and customization for end users. + +- **Single Responsibility**: Each L3 construct should solve a specific, well-defined problem or use case. Avoid creating "Swiss Army knife" constructs that try to handle multiple distinct scenarios. + +- **Limited Scope**: If a construct requires more than 5-7 configuration properties or creates more than 6-8 underlying resources, consider breaking it down into smaller, more focused constructs. + +- **Clear Boundaries**: The construct's purpose and limitations should be immediately apparent from its name and interface. Users shouldn't need to understand complex internal implementations to use it effectively. + +Consider breaking down a construct when: +- It has too many optional configuration parameters +- It implements multiple distinct patterns or architectural solutions +- Users frequently need to understand its internals to use it effectively +- The construct's documentation becomes too complex to maintain **The Minimal Deployable Pattern Definition should be minimal** -While a client should have great flexibility to fully specify how a Construct is deployed, the minimal deployment should consist of a single new statement. At times things like table schema might require some additional code – but if your minimal deployable solution is more than a couple lines long or requires creating one or more services first then your construct is outside the patterns for the library. +When designing a construct, the minimal configuration required to deploy it should be as small as possible. This means that default values should be provided for most properties, and only the essential parameters should be mandatory. + +- Required properties should be limited to those absolutely necessary for the construct to function +- All other properties should have sensible defaults +- Default values should follow AWS best practices +- The minimal configuration should result in a secure and functional deployment + +In simpler terms, you can think of it as "Be opinionated by default, flexible when required". + +Example: + +✅ Good: + +``` +// Only requires essential parameters +new ApiGateway(this, 'Api', { + handler: myLambdaFunction // Only the handler is required +}); +``` + +❌ Avoid: +``` +// Too many required parameters +new ApiGateway(this, 'Api', { + handler: myLambdaFunction, + stageName: 'prod', // Should have a default + cors: true, // Should have a default + authorization: 'IAM', // Should have a default + logRetention: 30 // Should have a default +}); +``` + +There is a possibility that the client could specify some props values that are incompatible with default props values. That is the responsibility of the client – if they choose to not use the default implementation then they take responsibility for the configuration they specify. **Business Logic** -The Construct don't have to be restricted to deploying infrastructure (eg – implemented Lambda functions). This is because we want to abstract the complexity of the underlying Generative AInology area. However, the business logic should be configurable to adapt to different use cases. +The Construct don't have to be restricted to deploying infrastructure (eg – implemented Lambda functions). This is because we want to abstract the complexity of the underlying Generative AI workflow area. However, the business logic should be configurable to adapt to different use cases. **Favor L2 CDK Constructs over L1** @@ -62,16 +107,5 @@ For the Typescript class name, use Pascal casing and concatenate the names separ Once again, these rules became clear as we wrote all of the existing constructs and not all of the early constructs comply with these rules. -# Service Usage in Constructs -It's important for consistency that services are implemented consistently across Generative AI Constructs – that clients specify the same properties. This section specifies the require attributes on your Construct Props interface for each service currently in the library, as well as the reasons behind any exceptions. We are unlikely to approve a construct with additional attributes, although we may if the proposed new attribute is appropriate for us to implement across all constructs using that service. - -If you are creating a construct that uses a service for the first time, defining the appropriate interface is a key step and we will work with you. - -This version of the document also lists what Constructs currently violate these standards and whether making the object compliant would be a breaking change. - -Existing Inconsistencies would not be published, that’s for our internal use – only the Required Properties and Attributes on Props would be published as requirements (along with specific notes). - -This section evolves as we add new constructs and use new services. - *** © Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. \ No newline at end of file diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index d6f5206f..bf491f83 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -51,7 +51,7 @@ npm install -g npm aws-cdk yarn projen ## Testing -AWS Generative AI CDK Constructs use two types of testing: unit testing and integration testing. Unit testing targets specific aspects of a construct or one of the functions in the core library. It examines the results and confirms the correct resources are there. For instance, it may call the deployLambdaFunction() in the core library and then confirm that AWS_NODEJS_CONNECTION_REUSE_ENABLED environment variable was set correctly. The unit tests check that certain aspects of the results are correct. You can learn more about unit testing CDK constructs [here](https://docs.aws.amazon.com/cdk/latest/guide/testing.html) and [here](https://aws.amazon.com/blogs/developer/testing-infrastructure-with-the-aws-cloud-development-kit-cdk/). +AWS Generative AI CDK Constructs use two types of testing: unit testing and integration testing. Unit testing targets specific aspects of a construct or one of the functions in the core library. The unit tests check that certain aspects of the results are correct. You can learn more about unit testing CDK constructs [here](https://docs.aws.amazon.com/cdk/latest/guide/testing.html) and [here](https://aws.amazon.com/blogs/developer/testing-infrastructure-with-the-aws-cloud-development-kit-cdk/). All test files can be found in the /test directory under each construct (or core). You'll find two types of files in this directory: @@ -78,13 +78,14 @@ All test files can be found in the /test directory under each construct (or core Navigate to the [Generative AI CDK Construct Repository] (https://github.com/aws-samples/generative-ai-cdk-constructs-samples): Open your command line interface and change directory to the generative AI CDK construct repository. -- Execute the command ```yarn build```. +- Execute the command ```projen build```. - This command runs npx projen build and generates a .jsii file in your repository. ### Step 2: Packaging the Constructs -1. Run Yarn Package:JS: - - Execute ```yarn package:js```. +1. Run ```projen package:js```: + - Execute ```projen package:js```. - This command creates a new .tgz package of all constructs in the dist/js folder. + - If you want to build a package for a different language, please refer to the existing ```projen package:X``` commands after running ```projen --help``` 2. Locate the TGZ File: - Find the generated .tgz file, typically named something like dist/js/generative-ai-cdk-constructs-0.0.0.jsii.tgz. diff --git a/README.md b/README.md index 4ddb6b1b..55078feb 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ - [Contributors](#contributors) - [Operational Metrics Collection](#operational-metrics-collection) - [Roadmap](#roadmap) +- [Deprecation](#deprecation) - [License](#license) - [Legal Disclaimer](#legal-disclaimer) @@ -180,6 +181,10 @@ Generative AI CDK Constructs may collect anonymous operational metrics, includin Roadmap is available through the [GitHub Project](https://github.com/orgs/awslabs/projects/136) +## Deprecation + +To understand our deprecation process, please refer to the dedicated [documentation](./DEPRECATION_PROCESS.md) + ## License Apache-2.0 diff --git a/projenrc/github-workflows.ts b/projenrc/github-workflows.ts index 3c8744ef..b6c61c93 100644 --- a/projenrc/github-workflows.ts +++ b/projenrc/github-workflows.ts @@ -554,7 +554,7 @@ export function buildCodeGenerationWorkflow(project: AwsCdkConstructLibrary) { { name: 'Create Pull Request', id: 'create-pr', - uses: 'peter-evans/create-pull-request@v4', + uses: 'peter-evans/create-pull-request@v7.0.6', with: { 'token': '${{ secrets.PROJEN_GITHUB_TOKEN }}', 'commit-message': [