-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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(core): Size.bytes() #24136
feat(core): Size.bytes() #24136
Conversation
There was a problem hiding this 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.
A comment requesting an exemption should contain the text Exemption Request
. Additionally, if clarification is needed add Clarification Request
to a comment.
/** | ||
* A nullable integer that is used to enable compression (with non-negative | ||
* between 0 and 10485760 (10M) bytes, inclusive) or disable compression | ||
* (when undefined) on an API. When compression is enabled, compression or | ||
* decompression is not applied on the payload if the payload size is | ||
* smaller than this value. Setting it to zero allows compression for any | ||
* payload size. | ||
* | ||
* @default - Compression is disabled. | ||
*/ | ||
readonly minimumCompressionSize?: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! This change wasn't supposed to be in this PR :)
@@ -648,6 +660,7 @@ export class SpecRestApi extends RestApiBase { | |||
name: this.restApiName, | |||
policy: props.policy, | |||
failOnWarnings: props.failOnWarnings, | |||
minimumCompressionSize: props.minimumCompressionSize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nor this one.
|
||
test('SpecRestApi minimumCompressionSize test', () => { | ||
// GIVEN | ||
const app = new App({ | ||
context: { | ||
'@aws-cdk/aws-apigateway:disableCloudWatchRole': true, | ||
}, | ||
}); | ||
|
||
const stack = new Stack(app); | ||
const api = new apigw.SpecRestApi(stack, 'SpecRestApi', { | ||
apiDefinition: apigw.ApiDefinition.fromInline({ foo: 'bar' }), | ||
minimumCompressionSize: 10485760, | ||
}); | ||
|
||
// WHEN | ||
api.root.addMethod('GET'); | ||
|
||
// THEN | ||
Template.fromStack(stack).hasResourceProperties('AWS::ApiGateway::RestApi', { | ||
Name: 'SpecRestApi', | ||
MinimumCompressionSize: 10485760, | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nor this one.
packages/@aws-cdk/core/lib/size.ts
Outdated
public static readonly Kibibytes = new StorageUnit('kibibytes', 1); | ||
public static readonly Bytes = new StorageUnit('bytes', 1 / 1024); | ||
public static readonly Mebibytes = new StorageUnit('mebibytes', 1024); | ||
public static readonly Gibibytes = new StorageUnit('gibibytes', 1024 * 1024); | ||
public static readonly Tebibytes = new StorageUnit('tebibytes', 1024 * 1024 * 1024); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 1 / 1024
makes me uneasy. Computers aren't good at representing fractional numbers and getting math with them correct.
Can we shift all fractions up by 1024
? Make bytes = 1
, kibibytes = 1024
, etc ?
const size = Size.bytes(1_073_741_823); | ||
expect(size.toBytes()).toEqual(1_073_741_823); | ||
|
||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple more tests here to be sure. Especially conversions between bytes and other units will be interesting.
✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
18a15e2
to
f7c5bac
Compare
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 CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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). |
This PR adds Size.bytes(), which allows to specify a Size class from an amount of bytes. Within the Size class,
Size.bytes( )
is the additional method added here to enable support for bytes as well as conversion to bytes withSize.toBytes
.For example,
creates a new object of the Size class with a size of 1024 bytes and which is equivalent to 1 kibibyte.
Closes #24106.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license