feat(ivs-alpha): support advanced channel type by mazyu36 · Pull Request #30086 · aws/aws-cdk

mazyu36 · 2024-05-07T10:09:09Z

Issue # (if applicable)

Closes #30075

Reason for this change

As described in the issue.

Description of changes

Add ADVANCED_HD and ADVANCED_SD to the ivs.ChannelType.
Add preset property to the Channel Construct. Additionally, validation has been implemented to allow setting the preset property only when using the Advanced channel type

Description of how you validated changes

Add both unit tests and integ tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

aws-cdk-automation

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.

nmussy

Mostly documentation nits, LGTM otherwise 👍

packages/@aws-cdk/aws-ivs-alpha/README.md

nmussy · 2024-05-07T12:01:09Z

packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

@@ -69,6 +70,42 @@ export enum ChannelType {
   * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ivs-channel.html


These @see tags should be moved to the enum TSDoc, no need to repeat them if they're pointing to the same link

@nmussy
Thank you for your review!
I moved @see to the enum TSDoc.

nmussy · 2024-05-07T12:01:29Z

packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

+export enum Preset {
+  /**
+   * Use a lower bitrate than STANDARD for each quality level. Use it if you have low download bandwidth and/or simple video content (e.g., talking heads).
+   * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ivs-channel.html


Again, @see should be moved to the Preset TSDoc

Likewise, I moved to the enum docs.

packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

nmussy · 2024-05-07T12:10:52Z

packages/@aws-cdk/aws-ivs-alpha/test/integ.ivs-channel-advanced.ts

+new IntegTest(app, 'ivs-test', {
+  testCases: [stack],
+});


You can run some assertions to make sure that the configurations are created properly, see integ-tests. Let me know if you need help setting them up

@nmussy
I added assertions to the integ tests.

Co-authored-by: Jimmy Gaussen <jimmy.gaussen@gmail.com>

mazyu36 · 2024-05-07T16:08:06Z

packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

      latencyMode: props.latencyMode,
      name: this.physicalName,
      type: props.type,
+      preset: props.preset,


Taking into account the revisions after the review, I changed it to set props.preset directly.
This is because if ChannelType is ADVANCED_SD or ADVANCED_HD, even if preset becomes undefined, Cfn will set the default value.

Once again, you can use the shorthand:

Suggested change

preset: props.preset,

preset,

Thank you.
I had a misunderstanding about the shorthand.

Uh no, I'm the one who made the mistake here, it doesn't work sorry 😅

@nmussy
No problem. I reverted it.

nmussy

Just a few more nits, looks good otherwise 👍

packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

nmussy · 2024-05-07T17:09:05Z

packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

      latencyMode: props.latencyMode,
      name: this.physicalName,
      type: props.type,
+      preset: props.preset,


Once again, you can use the shorthand:

Suggested change

preset: props.preset,

preset,

nmussy · 2024-05-07T17:09:32Z

packages/@aws-cdk/aws-ivs-alpha/test/integ.ivs-channel-advanced.ts

+})
+  .expect(integ.ExpectedResult.objectLike({
+    channel: {
+      preset: 'HIGHER_BANDWIDTH_DELIVERY',


This is great, we're testing the Cfn default value here 👍

Co-authored-by: Jimmy Gaussen <jimmy.gaussen@gmail.com>

This reverts commit 7ca984a.

packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

mergify · 2024-05-24T17:50:52Z

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-cdk-automation · 2024-05-24T18:20:01Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: b4de0ba
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2024-05-24T18:20:21Z

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#30075 ### Reason for this change As described in the issue. ### Description of changes * Add `ADVANCED_HD` and `ADVANCED_SD` to the `ivs.ChannelType`. * Add `preset` property to the Channel Construct. Additionally, validation has been implemented to allow setting the preset property only when using the Advanced channel type ### Description of how you validated changes Add both unit tests and integ tests. ### 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*

aws-cdk-automation · 2024-07-25T21:10:51Z

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.

support advanced channel type

c53f546

aws-cdk-automation requested a review from a team May 7, 2024 10:09

github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 labels May 7, 2024

aws-cdk-automation previously requested changes May 7, 2024

View reviewed changes

mazyu36 changed the title ~~(ivs-alpha): support advanced channel type~~ feat(ivs-alpha): support advanced channel type May 7, 2024

fix: README

c092956

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 7, 2024

nmussy suggested changes May 7, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 7, 2024

mazyu36 and others added 6 commits May 7, 2024 21:23

Update packages/@aws-cdk/aws-ivs-alpha/README.md

514da1d

Co-authored-by: Jimmy Gaussen <jimmy.gaussen@gmail.com>

Update packages/@aws-cdk/aws-ivs-alpha/README.md

6f66e10

Co-authored-by: Jimmy Gaussen <jimmy.gaussen@gmail.com>

Update packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

f509cd9

Co-authored-by: Jimmy Gaussen <jimmy.gaussen@gmail.com>

Update packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

bbda1ff

Co-authored-by: Jimmy Gaussen <jimmy.gaussen@gmail.com>

Update packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

deb8909

Co-authored-by: Jimmy Gaussen <jimmy.gaussen@gmail.com>

fix: bug, docs and integ test

1c24353

mazyu36 commented May 7, 2024

View reviewed changes

nmussy suggested changes May 7, 2024

View reviewed changes

mazyu36 and others added 5 commits May 8, 2024 02:44

Update packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

7ca984a

Co-authored-by: Jimmy Gaussen <jimmy.gaussen@gmail.com>

Update packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

23b89b3

Co-authored-by: Jimmy Gaussen <jimmy.gaussen@gmail.com>

Update packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts

b8866ed

Co-authored-by: Jimmy Gaussen <jimmy.gaussen@gmail.com>

Revert

323a833

This reverts commit 7ca984a.

fix bug

4536c74

nmussy approved these changes May 8, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 8, 2024

shikha372 self-assigned this May 10, 2024

shikha372 reviewed May 10, 2024

View reviewed changes

packages/@aws-cdk/aws-ivs-alpha/lib/channel.ts Outdated Show resolved Hide resolved

github-actions bot mentioned this pull request May 13, 2024

Weekly PR metrics report #30176

Closed

fix: overwrite preset when standard or basic chosen

275b495

mazyu36 force-pushed the ivs-advanced-channel-30075 branch from 60e2606 to 275b495 Compare May 22, 2024 00:55

Merge branch 'main' into ivs-advanced-channel-30075

e09ae34

shikha372 approved these changes May 24, 2024

View reviewed changes

Merge branch 'main' into ivs-advanced-channel-30075

b4de0ba

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 24, 2024

mergify bot merged commit 544e54a into aws:main May 24, 2024

mazyu36 deleted the ivs-advanced-channel-30075 branch May 25, 2024 02:14

github-actions bot mentioned this pull request Jun 1, 2024

Monthly PR metrics report #30414

Closed

NOUIY mentioned this pull request Jun 22, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#105

Open

kazimanzurrashid mentioned this pull request Jun 29, 2024

[Snyk] Upgrade aws-cdk-lib from 2.144.0 to 2.145.0 kazimanzurrashid/aws-scheduler-go#821

Open

This was referenced Jun 29, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#106

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#107

Open

This was referenced Jul 12, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#108

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#109

Open

This was referenced Jul 19, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#110

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#111

Open

aws locked as resolved and limited conversation to collaborators Jul 25, 2024

		@@ -69,6 +70,42 @@ export enum ChannelType {
		* @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ivs-channel.html

Conversation

mazyu36 commented May 7, 2024

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

nmussy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mazyu36 May 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nmussy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mergify bot commented May 24, 2024

Uh oh!

aws-cdk-automation commented May 24, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented May 24, 2024

Uh oh!

aws-cdk-automation commented Jul 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mazyu36 May 7, 2024 •

edited

Loading