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

(apigateway): support for minimumCompressionSize #22926

Closed
1 of 2 tasks
drissamri opened this issue Nov 15, 2022 · 7 comments · Fixed by #24067
Closed
1 of 2 tasks

(apigateway): support for minimumCompressionSize #22926

drissamri opened this issue Nov 15, 2022 · 7 comments · Fixed by #24067
Assignees
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@drissamri
Copy link

drissamri commented Nov 15, 2022

Describe the feature

SpecRestApi doesn't support minimumCompressionSize, while RestApi does

Use Case

We generate our API's based off a Open API specification and would like to use compression on our APIs

Proposed Solution

No response

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.50

Environment details (OS name and version, etc.)

OSX

@drissamri drissamri added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Nov 15, 2022
@drissamri drissamri changed the title (module name): (short issue description) (apigateway): support for minimumCompressionSize Nov 15, 2022
@github-actions github-actions bot added the @aws-cdk/aws-apigateway Related to Amazon API Gateway label Nov 15, 2022
@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Nov 15, 2022
@peterwoodworth
Copy link
Contributor

peterwoodworth commented Nov 15, 2022

Thanks for the request. Assuming there's no barrier to adding this prop to the SpecRestApi, this should be pretty simple to add to our SpecRestApi props, and then passing it in here

const resource = new CfnRestApi(this, 'Resource', {

Before we support this, you can use escape hatches to make use of the property:

const specRestApi = new SpecRestApi(...);
(specRestApi.node.defaultChild as CfnRestApi).addPropertyOverride('MinimumCompressionSize', x);

I am marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

Check out our contributing guide if you're interested in contributing yourself - there's a low chance the team will be able to address this soon but we'd be happy to review a PR 🙂

@pahud
Copy link
Contributor

pahud commented Nov 16, 2022

Hi @drissamri

Are you interested to pick up with the PR against this issue? I would be happy to help you get started. Feel free to reach out to me on cdk.dev slack.

@virajmavani
Copy link

I would like to take up this issue and contribute the fix. Please let me know if you can assign it. :)

@atchla
Copy link

atchla commented Dec 27, 2022

The linked PR looks like it was closed due to inactivity. Is this something I can pick up and finish off?

@peterwoodworth
Copy link
Contributor

Looks like the issue is yours for the taking @atchla! Go ahead 🙂

@mergify mergify bot closed this as completed in #24067 Mar 1, 2023
mergify bot pushed a commit that referenced this issue Mar 1, 2023
This PR adds minCompressionSize to SpecRestApi, which allows compression on api's payload. Within SpecRestApi ``` minComppresssionSize: Size``` is the additional prop added to SpecRestApiProps to support minCompressionSize  which takes class Size, the value of how much the payload needs to be compressed. 

This PR  also adds minCompressionSize to RestApi which supports  Size class e.g ``` minComppresssionSize: Size``` and deprecates minimumCompressionSize which has number as a type.

For example,
```ts
const api = new apigw.RestApi(stack, 'RestApi', {
      minCompressionSize: Size.bytes(1024),
    });
```
compression for the payload would be 1024 bytes which is equivalent to 1 kibibyte.

Closes #22926.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Mar 1, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@atchla
Copy link

atchla commented Mar 1, 2023

I had some things come up and didn't have enough bandwidth to take this task any more. Thanks for finishing it out @pattasai !

homakk pushed a commit to homakk/aws-cdk that referenced this issue Mar 28, 2023
This PR adds minCompressionSize to SpecRestApi, which allows compression on api's payload. Within SpecRestApi ``` minComppresssionSize: Size``` is the additional prop added to SpecRestApiProps to support minCompressionSize  which takes class Size, the value of how much the payload needs to be compressed. 

This PR  also adds minCompressionSize to RestApi which supports  Size class e.g ``` minComppresssionSize: Size``` and deprecates minimumCompressionSize which has number as a type.

For example,
```ts
const api = new apigw.RestApi(stack, 'RestApi', {
      minCompressionSize: Size.bytes(1024),
    });
```
compression for the payload would be 1024 bytes which is equivalent to 1 kibibyte.

Closes aws#22926.

----

*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
@aws-cdk/aws-apigateway Related to Amazon API Gateway effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants