feat(codebuild): allow setting the Project's logging configuration #11444

wulfmann · 2020-11-12T15:50:12Z

Fixes: #3856

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

gitpod-io · 2020-11-12T15:50:17Z

mergify · 2020-11-12T15:51:43Z

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

skinny85

Thanks for the contribution @wulfmann !

I'd like to see the used classes changed a little bit (including their names), but looks pretty solid!

packages/@aws-cdk/aws-codebuild/lib/project.ts

Pull request has been modified.

wulfmann · 2020-11-16T14:51:23Z

@skinny85 I've updated the changes you requested.

One thing i'm curious about is how would I implement a helper to allow for both of these log configurations? Would I add another class that implements ProjectLogs like the others? A config can have both s3 and cloudwatch configurations.

skinny85 · 2020-11-16T19:16:33Z

One thing i'm curious about is how would I implement a helper to allow for both of these log configurations? Would I add another class that implements ProjectLogs like the others? A config can have both s3 and cloudwatch configurations.

Oh shit. Is that really allowed?

wulfmann · 2020-11-16T22:58:35Z

Oh shit. Is that really allowed?

Yep, see the logs config section here

skinny85 · 2020-11-16T23:38:47Z

Damn. I was not aware of that. I thought you could only have a single type of logs active in a given Project. In that case, your original design might be better 😕.

Would you mind going back to the firs revision of this R?

Apologies for the churn ❤️.

wulfmann · 2020-11-17T00:54:00Z

Sure I’ll go back. In thinking about this, we might as well allow the enabled/disabled flag too. This might be the use case:

Cloudwatch logging is enabled by default so if you only wanted to send to s3, you would have to pass in the cloudwatch block with disabled.

Let me know your thoughts on that!

skinny85 · 2020-11-17T01:23:21Z

Cloudwatch logging is enabled by default so if you only wanted to send to s3, you would have to pass in the cloudwatch block with disabled.

Let me know your thoughts on that!

Sure, but kill the enum. Just make it an enabled?: boolean property, which defaults to true.

skinny85 · 2020-11-17T01:26:41Z

Also, thanks!

…led prop

wulfmann · 2020-11-17T13:35:22Z

Ok should be good to go for another review!

skinny85

Thanks for the contribution @wulfmann ! Looks very good. Some comments below.

packages/@aws-cdk/aws-codebuild/lib/project-logs.ts

packages/@aws-cdk/aws-codebuild/lib/project.ts

packages/@aws-cdk/aws-codebuild/package.json

Pull request has been modified.

wulfmann · 2020-11-18T13:13:08Z

@skinny85 Alright I've made the corrections you suggested. Let me know if you see anything else that should be changed!

skinny85

We're almost there @wulfmann , thanks for your patience!

The biggest thing is the naming: make sure all places use camelCase, so CloudWatch, not Cloudwatch.

Thanks!

packages/@aws-cdk/aws-codebuild/lib/project.ts

packages/@aws-cdk/aws-codebuild/lib/project-logs.ts

packages/@aws-cdk/aws-codebuild/README.md

packages/@aws-cdk/aws-codebuild/lib/project-logs.ts

packages/@aws-cdk/aws-codebuild/lib/project.ts

Pull request has been modified.

wulfmann · 2020-11-19T15:36:07Z

@skinny85 Alright ready for another round. I appreciate your thoroughness!!

skinny85

Looks fantastic. Thanks for the great contribution @wulfmann , and for your patience in dealing with my barrage of comments! 😬

mergify · 2020-11-19T22:35:43Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2020-11-19T23:30:07Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3ceba4e
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2020-11-19T23:30:28Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

In aws#11444, we added the option of setting the logging configuration of the Project. Unfortuantely, the Project's Role was not given permissions to write into that destination, which means executing the Project's build would fail. Fixes aws#12179

…2213) In #11444, we added the option of setting the logging configuration of the Project. Unfortuantely, the Project's Role was not given permissions to write into that destination, which means executing the Project's build would fail. Fixes #12179 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

…s#12213) In aws#11444, we added the option of setting the logging configuration of the Project. Unfortuantely, the Project's Role was not given permissions to write into that destination, which means executing the Project's build would fail. Fixes aws#12179 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

update tests and add log config

7971a6d

fix linting issues, update tests, fix docs

4abf28f

wulfmann mentioned this pull request Nov 12, 2020

CodeBuild: Add support for logging configuration in Project #3856

Closed

2 tasks

wulfmann changed the title ~~feat(codebuild): add logsConfig option~~ feat(aws-codebuild): add logsConfig option Nov 12, 2020

update readme

166f9ea

skinny85 previously requested changes Nov 12, 2020

View reviewed changes

packages/@aws-cdk/aws-codebuild/lib/project.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-codebuild/lib/project.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-codebuild/lib/project.ts Outdated Show resolved Hide resolved

SomayaB assigned skinny85 Nov 13, 2020

zxkane mentioned this pull request Nov 13, 2020

[codebuild] support LogsConfig and QueuedTimeoutInMinutes in L2 construct #11364

Closed

2 tasks

wulfmann force-pushed the feat-codebuild-logs branch from d79646d to 166f9ea Compare November 17, 2020 13:19

wulfmann added 3 commits November 17, 2020 07:20

Merge branch 'master' into feat-codebuild-logs

23a24e8

add a new test, move interfaces to new file, remove enum and add enab…

d80655b

…led prop

Merge branch 'master' into feat-codebuild-logs

370f897

skinny85 previously requested changes Nov 17, 2020

View reviewed changes

Merge branch 'master' into feat-codebuild-logs

5c321ef

wulfmann added 2 commits November 18, 2020 13:10

refactor some names, add another test, update readme

eefc8dd

fix a couple missed things

308dc89

Merge branch 'master' into feat-codebuild-logs

e0bbcd0

skinny85 previously requested changes Nov 19, 2020

View reviewed changes

Merge branch 'master' into feat-codebuild-logs

b59523f

wulfmann added 2 commits November 19, 2020 15:33

refactor logging names, update tests/readme, fix cloudwatch naming

f136499

add separator to s3 location and update tests

cb023b0

wulfmann changed the title ~~feat(aws-codebuild): add logsConfig option~~ feat(aws-codebuild): add logging options Nov 19, 2020

skinny85 approved these changes Nov 19, 2020

View reviewed changes

skinny85 changed the title ~~feat(aws-codebuild): add logging options~~ feat(codebuild): allow setting the Project's logging configuration Nov 19, 2020

Merge branch 'master' into feat-codebuild-logs

3ceba4e

mergify bot merged commit 6a4b22d into aws:master Nov 19, 2020

skinny85 mentioned this pull request Dec 23, 2020

fix(codebuild): Project lacks permissions to its log destinations #12213

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(codebuild): allow setting the Project's logging configuration #11444

feat(codebuild): allow setting the Project's logging configuration #11444

wulfmann commented Nov 12, 2020 •

edited

Loading

gitpod-io bot commented Nov 12, 2020 •

edited

Loading

mergify bot commented Nov 12, 2020

skinny85 left a comment

wulfmann commented Nov 16, 2020

skinny85 commented Nov 16, 2020

wulfmann commented Nov 16, 2020 •

edited

Loading

skinny85 commented Nov 16, 2020

wulfmann commented Nov 17, 2020

skinny85 commented Nov 17, 2020 •

edited

Loading

skinny85 commented Nov 17, 2020

wulfmann commented Nov 17, 2020

skinny85 left a comment

wulfmann commented Nov 18, 2020

skinny85 left a comment

wulfmann commented Nov 19, 2020

skinny85 left a comment

mergify bot commented Nov 19, 2020

aws-cdk-automation commented Nov 19, 2020

mergify bot commented Nov 19, 2020

feat(codebuild): allow setting the Project's logging configuration #11444

feat(codebuild): allow setting the Project's logging configuration #11444

Conversation

wulfmann commented Nov 12, 2020 • edited Loading

gitpod-io bot commented Nov 12, 2020 • edited Loading

mergify bot commented Nov 12, 2020

skinny85 left a comment

Choose a reason for hiding this comment

wulfmann commented Nov 16, 2020

skinny85 commented Nov 16, 2020

wulfmann commented Nov 16, 2020 • edited Loading

skinny85 commented Nov 16, 2020

wulfmann commented Nov 17, 2020

skinny85 commented Nov 17, 2020 • edited Loading

skinny85 commented Nov 17, 2020

wulfmann commented Nov 17, 2020

skinny85 left a comment

Choose a reason for hiding this comment

wulfmann commented Nov 18, 2020

skinny85 left a comment

Choose a reason for hiding this comment

wulfmann commented Nov 19, 2020

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Nov 19, 2020

aws-cdk-automation commented Nov 19, 2020

AWS CodeBuild CI Report

mergify bot commented Nov 19, 2020

wulfmann commented Nov 12, 2020 •

edited

Loading

gitpod-io bot commented Nov 12, 2020 •

edited

Loading

wulfmann commented Nov 16, 2020 •

edited

Loading

skinny85 commented Nov 17, 2020 •

edited

Loading