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

API Gateway Compression #676

Merged
merged 3 commits into from
Jun 7, 2019
Merged

API Gateway Compression #676

merged 3 commits into from
Jun 7, 2019

Conversation

nplutt
Copy link
Contributor

@nplutt nplutt commented Jan 11, 2018

Completes feature for issue #672.

I was disappointed that there is no way to do this using the API Gateway Extensions to Swagger, so I had to use the update_rest_api method and set the compression level after the API Gateway is deployed.

The new functionality can is enabled by using an optional configuration parameter of minimum_compression_size in the config.json file. The parameter can be set at either the global level or the stage level and if both exist the stage level overrides the global level. Below is an example:

{
  "minimum_compression_size": 0,
  "stages": {
    "dev": {
      "api_gateway_stage": "api"
    },
    "test": {
      "api_gateway_stage": "test",
      "minimum_compression_size": 1000
    }
  },
  "version": "2.0",
  "app_name": "compression"
}

In this example dev would have a minimum_compression_size of 0 and test would have a minimum_compression_size of 1000.

Let me know if you would like anything changed, thanks!

@nplutt
Copy link
Contributor Author

nplutt commented Jan 18, 2018

Hey, just wanted to follow up on this. Let me know if you see any issues or want anything else added/changed.

Thanks!

@nplutt
Copy link
Contributor Author

nplutt commented Jan 18, 2018

Please wait on this PR, I just saw that the API gateway swagger now supports compression with a value of x-amazon-apigateway-minimum-compression-size and will be updating the PR accordingly.

@nplutt
Copy link
Contributor Author

nplutt commented Jan 25, 2018

So, unfortunately it looks like there is a bug in the AWS api. Using the x-amazon-apigateway-minimum-compression-size in the swagger doc to create an API gateway works but when the value is changed to a different number the api gateway is not updating. Below are steps to reproduce:

>>> import boto3
>>> import json
>>> client = boto3.client('apigateway')
>>> swagger_doc_1 = {'info': {'version': '1.0', 'title': 'comp'}, 'paths': {'/': {'get': {'responses': {'200': {'description': '200 response', 'schema': {'$ref': '#/definitions/Empty'}}}, 'x-amazon-apigateway-integration': {'contentHandling': 'CONVERT_TO_TEXT', 'responses': {'default': {'statusCode': '200'}}, 'uri': 'arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:562628674386:function:comp-dev/invocations', 'httpMethod': 'POST', 'passthroughBehavior': 'when_no_match', 'type': 'aws_proxy'}, 'consumes': ['application/json'], 'produces': ['application/json']}}}, 'schemes': ['https'], 'x-amazon-apigateway-minimum-compression-size': 1000, 'x-amazon-apigateway-binary-media-types': ['application/octet-stream', 'application/x-tar', 'application/zip', 'audio/basic', 'audio/ogg', 'audio/mp4', 'audio/mpeg', 'audio/wav', 'audio/webm', 'image/png', 'image/jpg', 'image/gif', 'video/ogg', 'video/mpeg', 'video/webm'], 'definitions': {'Empty': {'type': 'object', 'title': 'Empty Schema'}}, 'swagger': '2.0'}
>>> response_1 = client.import_rest_api(body=json.dumps(swagger_doc_1, indent=2))
>>> rest_api_id = response_1['id']
>>> response_1['minimumCompressionSize']
>>> 1000
>>> response_1['ResponseMetadata']['HTTPStatusCode']
>>> 200
>>>
>>> swagger_doc_2 = {'info': {'version': '1.0', 'title': 'comp'}, 'paths': {'/': {'get': {'responses': {'200': {'description': '200 response', 'schema': {'$ref': '#/definitions/Empty'}}}, 'x-amazon-apigateway-integration': {'contentHandling': 'CONVERT_TO_TEXT', 'responses': {'default': {'statusCode': '200'}}, 'uri': 'arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:562628674386:function:comp-dev/invocations', 'httpMethod': 'POST', 'passthroughBehavior': 'when_no_match', 'type': 'aws_proxy'}, 'consumes': ['application/json'], 'produces': ['application/json']}}}, 'schemes': ['https'], 'x-amazon-apigateway-minimum-compression-size': 5000, 'x-amazon-apigateway-binary-media-types': ['application/octet-stream', 'application/x-tar', 'application/zip', 'audio/basic', 'audio/ogg', 'audio/mp4', 'audio/mpeg', 'audio/wav', 'audio/webm', 'image/png', 'image/jpg', 'image/gif', 'video/ogg', 'video/mpeg', 'video/webm'], 'definitions': {'Empty': {'type': 'object', 'title': 'Empty Schema'}}, 'swagger': '2.0'}
>>> response_2 = client.put_rest_api(restApiId=rest_api_id, mode='overwrite',
body=json.dumps(swagger_doc_2, indent=2))
>>> response_2['minimumCompressionSize']
>>> 1000
>>> response_2['ResponseMetadata']['HTTPStatusCode']
>>> 200

As you can see, the first time the gateway is created with a minimum compression size of 1000 which works just fine. However the second time when the minimum compression size is changed to 5000 the api gateway doesn't update.

If you want to merge the current pull request that works using the boto update_rest_api functionality, that is fine with me. Also if you would like to wait for amazon to fix the bug, just let me know and I can put this pull request on hold for the time being.

Thanks!

@nplutt
Copy link
Contributor Author

nplutt commented Mar 13, 2018

Hey, just wanted to follow up on this. Let me know if you see any issues or want anything else added/changed.

Thanks!

@pickfire
Copy link
Contributor

pickfire commented Jul 2, 2018

Any news on this?

@gallmerci
Copy link

@nplutt I tried to reproduce your problem but for me it is working everything as expected. Maybe they fixed it?
Another comment: Can you also add the compression configuration to the swagger definition in the SAM file generated by chalice deploy? Or is this directly updated with your current implementation?

@nplutt
Copy link
Contributor Author

nplutt commented Aug 2, 2018

Thanks for the feedback @gallmerci. When I created this it was less than a week after they released the compression feature to API Gateway. I'm happy to hear that the issue that I was running into was fixed.

If you have the ability to merge this pull request I would be happy to make the changes. If not I'm probably not going to touch this, I have several pull requests that have been open since around January and none of them have recieved any feedback from any of the maintainers despite me reaching out multiple times.

@gallmerci
Copy link

@nplutt I would love to be able to do this, but unfortunately, I am not :(
I can just loudly vote for this idea!

@wutachih
Copy link

wutachih commented Jun 2, 2019

Is how the compression handled documented? I need to post a large json from an ETL process. The compression should help.

@stealthycoin stealthycoin force-pushed the compression branch 2 times, most recently from a400a47 to 820d119 Compare June 7, 2019 01:19
@stealthycoin stealthycoin requested review from jamesls and kyleknap June 7, 2019 01:19
Also added support for CloudFormation.
@codecov-io
Copy link

codecov-io commented Jun 7, 2019

Codecov Report

Merging #676 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #676      +/-   ##
==========================================
+ Coverage   95.54%   95.58%   +0.03%     
==========================================
  Files          27       27              
  Lines        4606     4645      +39     
  Branches      582      587       +5     
==========================================
+ Hits         4401     4440      +39     
  Misses        134      134              
  Partials       71       71
Impacted Files Coverage Δ
chalice/deploy/planner.py 96.31% <ø> (ø) ⬆️
chalice/awsclient.py 93.91% <100%> (+0.04%) ⬆️
chalice/deploy/deployer.py 99.17% <100%> (+0.04%) ⬆️
chalice/config.py 95.62% <100%> (+0.05%) ⬆️
chalice/constants.py 100% <100%> (ø) ⬆️
chalice/deploy/validate.py 100% <100%> (ø) ⬆️
chalice/package.py 100% <100%> (ø) ⬆️
chalice/deploy/models.py 99.33% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fae37c0...989eeec. Read the comment docs.

@stealthycoin
Copy link
Contributor

@nplutt Thanks for the submission. I rebased this off of the current state of the master branch and updated it to use the new deployer. Added support for cloudformation as well.

Copy link
Member

@JordonPhillips JordonPhillips left a comment

Choose a reason for hiding this comment

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

Looks good! Had a minor formatting thing but it's not a blocker.

@@ -146,6 +149,18 @@ def _validate_cors_for_route(route_url, route_methods):
)


def validate_minimum_compression_size(config):
# type: (Config) -> None
if config.minimum_compression_size is not None:
Copy link
Member

Choose a reason for hiding this comment

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

Not super important, but it would be nice to reduce the indentation here

Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Looks good just had a small comment

the minimum compression size to apply to the API gateway. If
this key is specified in both a stage specific config option
as well as a top level key, the stage specific key will
override the top level key for the given stage.
Copy link
Contributor

Choose a reason for hiding this comment

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

@stealthycoin stealthycoin merged commit d82d499 into aws:master Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants