-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(codebuild): Fleet
L2
#29754
feat(codebuild): Fleet
L2
#29754
Conversation
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.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request
. Additionally, if clarification is needed add Clarification Request
to a comment.
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
@@ -1,5 +1,7 @@ | |||
/** | |||
* Build machine compute type. | |||
* | |||
* @see https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-compute-types.html#environment.types |
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.
might be working combining this file, environment-type.ts
, and FleetComputeType
into an enums file?
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.
The current "convention" for this package is that most of the enums are part of their feature file (e.g. ReportGroupType
in report-group.ts
), with the exception of ComputeType
, which has its own file. I assume this was because it was used across multiple "features", in this case both Project
and IBuildImages
implementations, and now also Fleet
. EnvironmentType
is in a similar situation, used both for the IBuildImage
s and Fleet
classes.
If anything, the best thing to do would be to move both ComputeType
and EnvironmentType
in one of the feature files, probably project.ts
, which contains IBuildImage
?
LARGE = ComputeType.LARGE, | ||
X_LARGE = ComputeType.X_LARGE, | ||
X2_LARGE = ComputeType.X2_LARGE, | ||
} |
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.
see note above. worth moving all the enums into enums.ts
?
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.
See #29754 (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.
lgtm, just those doc changes and this is good to go
@comcalvi Nudge |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
### Issue # (if applicable) Closes aws#29617. ### Reason for this change Implementation of reserved capacity CodeBuild projects ### Description of changes * Add Fleet Construct * Add `EnvironmentType` enum * Refactor existing type strings to use the new enum * Validate that Windows 2022 build images can only be used in fleet projects Changes merged from aws#29616: * Added missing build images * Updated JSDoc comments to indicate AL2023 based images, see [docs](https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-available.html) * It might be a good idea to deprecate and rename `AMAZON_LINUX_2_STANDARD_3_0` to `AMAZON_LINUX_2023_STANDARD_3_0`, despite how the images are named. I'll leave it up to the maintainers * Added `{@link}` tags where missing ### Description of how you validated changes Unit and integ tests The images were retrieved using the [codebuild:ListCuratedEnvironmentImages](https://docs.aws.amazon.com/codebuild/latest/APIReference/API_ListCuratedEnvironmentImages.html) API command, and comparing it to the CDK. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
### Issue # (if applicable) Closes aws#29617. ### Reason for this change Implementation of reserved capacity CodeBuild projects ### Description of changes * Add Fleet Construct * Add `EnvironmentType` enum * Refactor existing type strings to use the new enum * Validate that Windows 2022 build images can only be used in fleet projects Changes merged from aws#29616: * Added missing build images * Updated JSDoc comments to indicate AL2023 based images, see [docs](https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-available.html) * It might be a good idea to deprecate and rename `AMAZON_LINUX_2_STANDARD_3_0` to `AMAZON_LINUX_2023_STANDARD_3_0`, despite how the images are named. I'll leave it up to the maintainers * Added `{@link}` tags where missing ### Description of how you validated changes Unit and integ tests The images were retrieved using the [codebuild:ListCuratedEnvironmentImages](https://docs.aws.amazon.com/codebuild/latest/APIReference/API_ListCuratedEnvironmentImages.html) API command, and comparing it to the CDK. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@nmussy I'm curious, is the L2 construct working then based off your pull request? It seems that you don't have to use a L1 construct but when deploying the fleet property is not being picked up by the build project |
@michaelbwebb I'm not sure what you're asking here, would you be able to open a new issue detailing what you're attempting to do, and include an example of your CDK code? Feel free to tag me there as well |
### Issue # (if applicable) Closes aws#29617. ### Reason for this change Implementation of reserved capacity CodeBuild projects ### Description of changes * Add Fleet Construct * Add `EnvironmentType` enum * Refactor existing type strings to use the new enum * Validate that Windows 2022 build images can only be used in fleet projects Changes merged from aws#29616: * Added missing build images * Updated JSDoc comments to indicate AL2023 based images, see [docs](https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-available.html) * It might be a good idea to deprecate and rename `AMAZON_LINUX_2_STANDARD_3_0` to `AMAZON_LINUX_2023_STANDARD_3_0`, despite how the images are named. I'll leave it up to the maintainers * Added `{@link}` tags where missing ### Description of how you validated changes Unit and integ tests The images were retrieved using the [codebuild:ListCuratedEnvironmentImages](https://docs.aws.amazon.com/codebuild/latest/APIReference/API_ListCuratedEnvironmentImages.html) API command, and comparing it to the CDK. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one. |
Issue # (if applicable)
Closes #29617.
Reason for this change
Implementation of reserved capacity CodeBuild projects
Description of changes
EnvironmentType
enumChanges merged from #29616:
AMAZON_LINUX_2_STANDARD_3_0
toAMAZON_LINUX_2023_STANDARD_3_0
, despite how the images are named. I'll leave it up to the maintainers{@link}
tags where missingDescription of how you validated changes
Unit and integ tests
The images were retrieved using the codebuild:ListCuratedEnvironmentImages API command, and comparing it to the CDK.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license