Skip to content
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(synthetics): lifecycle rules for auto-generated artifact buckets #22863

Closed

Conversation

mackensen
Copy link

@mackensen mackensen commented Nov 10, 2022

This change exposes the lifecycle rules property for new and existing auto-generated artifact buckets so that workload owners can easily manage growth.

fixes #22634

I'm not able to run the integration tests.


All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…buckets

This change exposes the lifecycle rules property for new and existing auto-generated artifact buckets so that workload owners can easily manage growth.

fixes aws#22634
@gitpod-io
Copy link

gitpod-io bot commented Nov 10, 2022

@github-actions github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Nov 10, 2022
@aws-cdk-automation aws-cdk-automation requested a review from a team November 10, 2022 15:09
@github-actions github-actions bot added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 labels Nov 10, 2022
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a 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.

@kaizencc kaizencc changed the title feat(synthetics): expose lifecycle rules for auto-generated artifact … feat(synthetics): lifecycle rules for auto-generated artifact buckets Nov 10, 2022
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mackensen! This looks good! I'll run the integ test sometime today. In the meantime, can you add this somewhere to the readme?

Comment on lines 92 to 96
* Lifecycle rules for the generated canary artifact bucket.
*
* @default - No rules applied. This has no effect if a bucket is passed
* to `artifactsBucketLocation`.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Lifecycle rules for the generated canary artifact bucket.
*
* @default - No rules applied. This has no effect if a bucket is passed
* to `artifactsBucketLocation`.
*/
* Lifecycle rules for the generated canary artifact bucket. Has no effect
* if a bucket is passed to `artifactsBucketLocation`. If you pass a bucket
* to `artifactsBucketLocation`, you can add lifecycle rules to the bucket
* itself.
*
* @default - no rules applied to the generated bucket.
*/

handler: 'index.handler',
code: synthetics.Code.fromInline('/* Synthetics handler code */'),
}),
runtime: synthetics.Runtime.SYNTHETICS_1_0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
runtime: synthetics.Runtime.SYNTHETICS_1_0,
runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_3_8,

@mackensen
Copy link
Author

@kaizencc thanks for the feedback, I've updated the branch.

@kaizencc
Copy link
Contributor

@mackensen I ran the integ test for you. It is somehow here. Not sure what went on to make it not a part of this particular branch, but Ill leave it to you to merge your branch with the commit that has the updated snapshot.

In addition, this PR needs a readme update that mentions the ability to add lifecycle rules to the created s3 bucket.

@mackensen
Copy link
Author

@kaizencc thanks, cherry-picked your commit and updated the README.

@aws-cdk-automation aws-cdk-automation dismissed their stale review November 12, 2022 23:12

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: e54e783
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@kaizencc kaizencc self-requested a review December 2, 2022 15:01
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mackensen! Apologies for not getting back to you sooner. This is ready to go once the build succeeds. My comments on the readme, when addressed, should fix the build.

enforceSSL: true,
});

const canary = new synthetics.Canary(this, 'MyCanary', {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the build is failing because we are creating two canary consts in the same example.


You can pass an S3 bucket to store artifacts from canary runs. If you do not, one will be auto-generated when the canary is created. You may add [lifecycle rules](https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-lifecycle-mgmt.html) to the auto-generated bucket.

Artifact bucket examples:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets delete this line and move it in to the ts example

Comment on lines +230 to +231
```typescript
// Auto-generated bucket with lifecycle rules.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```typescript
// Auto-generated bucket with lifecycle rules.
```ts
// configure lifecycle rules on the auto-generated artifact bucket.

Comment on lines +248 to +249

// Custom bucket as artifact bucket.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

github ui doesn't like when my suggestions involve "```" so I'll just describe what i want here.

we should split up the example into two here and change the comment to the one below:

// configure lifecycle rules on a custom artifact bucket.

@aws aws deleted a comment from aws-cdk-automation Dec 2, 2022
@aws-cdk-automation
Copy link
Collaborator

This PR has been in the BUILD FAILING state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

@aws-cdk-automation
Copy link
Collaborator

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

@aws-cdk-automation aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Dec 11, 2022
mergify bot pushed a commit that referenced this pull request Jul 11, 2023
…#26290)

A re-roll of #22863. Thanks for getting this started @mackensen!

This change exposes the lifecycle rules property for new and existing auto-generated artifact buckets so that workload owners can easily manage growth.

Fixes #22634

Co-authored-by:  Charles Fulton [email protected]

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
bmoffatt pushed a commit to bmoffatt/aws-cdk that referenced this pull request Jul 29, 2023
…aws#26290)

A re-roll of aws#22863. Thanks for getting this started @mackensen!

This change exposes the lifecycle rules property for new and existing auto-generated artifact buckets so that workload owners can easily manage growth.

Fixes aws#22634

Co-authored-by:  Charles Fulton [email protected]

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(synthetics): expose lifecycle rules for generated artifacts bucket
3 participants