-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Dev appplatform microsoft.app platform 2020 11 01 preview #13336
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
Changes from all commits
f3cc02f
acfbe38
4b010d0
9845b66
7254037
7e731aa
99a88e9
31133b1
ae88e43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,8 +9,10 @@ | |
| "artifactSelector": "sub-module-1" | ||
| }, | ||
| "deploymentSettings": { | ||
| "cpu": 1, | ||
| "memoryInGB": 3, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the removing of these properties intentional? I still see them in the deploymentSettings response model.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, it is intentional. According to current design, the users can use either cpu/memory or resourceRequests to represent his App's size. It is a straight either or, they don't need to set them all. |
||
| "resourceRequests": { | ||
| "cpu": "1000m", | ||
| "memory": "3Gi" | ||
| }, | ||
| "jvmOptions": "-Xms1G -Xmx3G", | ||
| "environmentVariables": { | ||
| "env": "test" | ||
|
|
@@ -46,6 +48,10 @@ | |
| "deploymentSettings": { | ||
| "cpu": 1, | ||
| "memoryInGB": 3, | ||
| "resourceRequests": { | ||
| "cpu": "1000m", | ||
| "memory": "3Gi" | ||
| }, | ||
| "jvmOptions": "-Xms1G -Xmx3G", | ||
| "environmentVariables": { | ||
| "env": "test" | ||
|
|
@@ -87,6 +93,10 @@ | |
| "deploymentSettings": { | ||
| "cpu": 1, | ||
| "memoryInGB": 3, | ||
| "resourceRequests": { | ||
| "cpu": "1000m", | ||
| "memory": "3Gi" | ||
| }, | ||
| "jvmOptions": "-Xms1G -Xmx3G", | ||
| "environmentVariables": { | ||
| "env": "test" | ||
|
|
@@ -128,6 +138,10 @@ | |
| "deploymentSettings": { | ||
| "cpu": 1, | ||
| "memoryInGB": 3, | ||
| "resourceRequests": { | ||
| "cpu": "1000m", | ||
| "memory": "3Gi" | ||
| }, | ||
| "jvmOptions": "-Xms1G -Xmx3G", | ||
| "environmentVariables": { | ||
| "env": "test" | ||
|
|
||
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.
As the property name this is not clear how the unit should be. I'd recommend making the name a bit clear and changing the type to integer. Please see an example of how both cpu and memory can be defined here:
azure-rest-api-specs/specification/compute/resource-manager/Microsoft.Compute/stable/2020-12-01/compute.json
Line 7609 in abfe5f8
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.
It is by design, we want to make cpu and memory field in our API align with the AKS Resource requests convention: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
For cpu, the user can pass 1 or 1000m to represent the 1c.
For memory, the user can pass 1024Mi or 1Gi to represent the 1Gb.
For now, our system needs to support the size like 0.5c/0.5Gb App, the previous cpu and memoryInGb properties are integer type, so they can't represent the 0.5c/0.5Gb size, so we introduce a new field resourceRequests into the API request.
The user can use ethier cpu/memory or resourceRequests to represent his App's size.
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.
I see - thanks for the explanation.