You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2020/10/13 17:40:00 UTC
[GitHub] [incubator-yunikorn-site] kingamarton opened a new pull request #29: [YUNIKORN-405] Document config update API
kingamarton opened a new pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] wilfred-s commented on a change in pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#discussion_r507643258
##########
File path: docs/api/scheduler.md
##########
@@ -433,6 +433,141 @@ Reponse
"reason": "yaml: unmarshal errors:\n line 7: cannot unmarshal !!str `wrong_text` into configs.PartitionConfig"
}
```
+## Configuration
+
+Endpoint to retrieve the current scheduler configuration
+
+**URL** : `/ws/v1/config`
+
+**Method** : `GET`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```json
+partitions:
+- name: default
+ queues:
+ - name: root
+ parent: true
+ submitacl: '*'
+ placementrules:
+ - name: tag
+ create: true
+ value: namespace
+checksum:
+- 186
+- 179
+- 215
+- 100
+- 2
+- 130
+- 126
+- 171
+- 230
+- 47
+- 167
+- 228
+- 198
+- 188
+- 244
+- 216
+- 221
+- 149
+- 82
+- 131
+- 69
+- 97
+- 182
+- 182
+- 96
+- 239
+- 55
+- 254
+- 217
+- 41
+- 151
+- 145
+
+sha256 checksum: BAB3D76402827EABE62FA7E4C6BCF4D8DD9552834561B6B660EF37FED9299791
Review comment:
That is the way the json marshaller does the `[]byte` marshalling. It converts every value in the array to a 8bit integer (0-255) and prints the value on a line. Unmarshalling probably gives you the right thing back again but it is not readable.
In the `getClusterConfig()` call I thus opted for a printed format using the standard base 16, upper-case, two characters per byte conversion.
The config also has ``` Checksum [32]byte `yaml:"-" json:"-"` ``` added so the marshaller leaves it alone. The yaml version is slightly different than what I added but still human readable. We can use the `TestGetConfigYAML` and `TestGetConfigJSON` to see the impact of the marshaller
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] yangwwei commented on a change in pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#discussion_r506063590
##########
File path: docs/api/scheduler.md
##########
@@ -433,6 +433,141 @@ Reponse
"reason": "yaml: unmarshal errors:\n line 7: cannot unmarshal !!str `wrong_text` into configs.PartitionConfig"
}
```
+## Configuration
+
+Endpoint to retrieve the current scheduler configuration
+
+**URL** : `/ws/v1/config`
+
+**Method** : `GET`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```json
+partitions:
+- name: default
+ queues:
+ - name: root
+ parent: true
+ submitacl: '*'
+ placementrules:
+ - name: tag
+ create: true
+ value: namespace
+checksum:
+- 186
+- 179
+- 215
+- 100
+- 2
+- 130
+- 126
+- 171
+- 230
+- 47
+- 167
+- 228
+- 198
+- 188
+- 244
+- 216
+- 221
+- 149
+- 82
+- 131
+- 69
+- 97
+- 182
+- 182
+- 96
+- 239
+- 55
+- 254
+- 217
+- 41
+- 151
+- 145
+
+sha256 checksum: BAB3D76402827EABE62FA7E4C6BCF4D8DD9552834561B6B660EF37FED9299791
Review comment:
Oh, no. I don't think this is correct.
@wilfred-s , could you please take a look at this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] kingamarton commented on a change in pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
kingamarton commented on a change in pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#discussion_r504617588
##########
File path: docs/api/scheduler.md
##########
@@ -434,6 +434,76 @@ Reponse
}
```
+## Configuration update
+
+Endpoint to override scheduler configuration.
+
+**URL** : `/ws/v1/config`
+
+**Method** : `PUT`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```yaml
+partitions:
+ -
+ name: default
+ placementrules:
+ - name: tag
+ value: namespace
+ create: true
+ queues:
+ - name: root
+ submitacl: '*'
+ properties:
+ application.sort.policy: stateaware
+checksum: [173,7,83,162,71,12,24,54,61,72,195,227,207,95,85,14,128,101,107,7,112,214,125,208,136,131,212,118,44,191,90,222]
+```
+**Note:** the checksum is the base configuration checksum on top of what we want to make the changes.
+If the provisioned checksum differs from the actual scheduler checksum, the configuration update will fail.
Review comment:
changed it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] yangwwei commented on a change in pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#discussion_r504312845
##########
File path: docs/api/scheduler.md
##########
@@ -434,6 +434,76 @@ Reponse
}
```
+## Configuration update
+
+Endpoint to override scheduler configuration.
+
+**URL** : `/ws/v1/config`
+
+**Method** : `PUT`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```yaml
+partitions:
+ -
+ name: default
+ placementrules:
+ - name: tag
+ value: namespace
+ create: true
+ queues:
+ - name: root
+ submitacl: '*'
+ properties:
+ application.sort.policy: stateaware
+checksum: [173,7,83,162,71,12,24,54,61,72,195,227,207,95,85,14,128,101,107,7,112,214,125,208,136,131,212,118,44,191,90,222]
+```
+**Note:** the checksum is the base configuration checksum on top of what we want to make the changes.
+If the provisioned checksum differs from the actual scheduler checksum, the configuration update will fail.
+
+### Failure response
+
+The configuration update can fail due to different reasons such as:
+- invalid configuration,
+- outdated base checksum,
+- failure while saving the configuration to the scheduler.
Review comment:
this line is quite confusing... I think this is to outline what could be the failures
this will be hard for users to understand why the saving fails..
##########
File path: docs/api/scheduler.md
##########
@@ -434,6 +434,76 @@ Reponse
}
```
+## Configuration update
+
+Endpoint to override scheduler configuration.
+
+**URL** : `/ws/v1/config`
+
+**Method** : `PUT`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```yaml
+partitions:
+ -
+ name: default
+ placementrules:
+ - name: tag
+ value: namespace
+ create: true
+ queues:
+ - name: root
+ submitacl: '*'
+ properties:
+ application.sort.policy: stateaware
+checksum: [173,7,83,162,71,12,24,54,61,72,195,227,207,95,85,14,128,101,107,7,112,214,125,208,136,131,212,118,44,191,90,222]
+```
+**Note:** the checksum is the base configuration checksum on top of what we want to make the changes.
+If the provisioned checksum differs from the actual scheduler checksum, the configuration update will fail.
Review comment:
> If the provisioned checksum differs from the actual scheduler checksum, the configuration update will fail.
if the provided checksum in the request differs from the base version, the configuration update will fail. This is to avoid applying any unintentional changes to the running version. Subsequently, the client needs to retrieve the latest version from the scheduler and send another update with the up-to-date checksum.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] kingamarton commented on a change in pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
kingamarton commented on a change in pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#discussion_r507777554
##########
File path: docs/api/scheduler.md
##########
@@ -433,6 +433,141 @@ Reponse
"reason": "yaml: unmarshal errors:\n line 7: cannot unmarshal !!str `wrong_text` into configs.PartitionConfig"
}
```
+## Configuration
+
+Endpoint to retrieve the current scheduler configuration
+
+**URL** : `/ws/v1/config`
+
+**Method** : `GET`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```json
+partitions:
+- name: default
+ queues:
+ - name: root
+ parent: true
+ submitacl: '*'
+ placementrules:
+ - name: tag
+ create: true
+ value: namespace
+checksum:
+- 186
+- 179
+- 215
+- 100
+- 2
+- 130
+- 126
+- 171
+- 230
+- 47
+- 167
+- 228
+- 198
+- 188
+- 244
+- 216
+- 221
+- 149
+- 82
+- 131
+- 69
+- 97
+- 182
+- 182
+- 96
+- 239
+- 55
+- 254
+- 217
+- 41
+- 151
+- 145
+
+sha256 checksum: BAB3D76402827EABE62FA7E4C6BCF4D8DD9552834561B6B660EF37FED9299791
Review comment:
Thank you @wilfred-s. I will need to find an another solution for the checksum marshalling problem for the validation, because this is not a user friendly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] kingamarton commented on a change in pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
kingamarton commented on a change in pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#discussion_r505721079
##########
File path: docs/api/scheduler.md
##########
@@ -434,6 +434,76 @@ Reponse
}
```
+## Configuration update
+
+Endpoint to override scheduler configuration.
+
+**URL** : `/ws/v1/config`
+
+**Method** : `PUT`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```yaml
+partitions:
+ -
+ name: default
+ placementrules:
+ - name: tag
+ value: namespace
+ create: true
+ queues:
+ - name: root
+ submitacl: '*'
+ properties:
+ application.sort.policy: stateaware
+checksum: [173,7,83,162,71,12,24,54,61,72,195,227,207,95,85,14,128,101,107,7,112,214,125,208,136,131,212,118,44,191,90,222]
+```
+**Note:** the checksum is the base configuration checksum on top of what we want to make the changes.
+If the provisioned checksum differs from the actual scheduler checksum, the configuration update will fail.
+
+### Failure response
+
+The configuration update can fail due to different reasons such as:
+- invalid configuration,
+- outdated base checksum,
+- failure while saving the configuration to the scheduler.
Review comment:
removed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] yangwwei commented on a change in pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#discussion_r504808597
##########
File path: docs/api/scheduler.md
##########
@@ -433,6 +433,141 @@ Reponse
"reason": "yaml: unmarshal errors:\n line 7: cannot unmarshal !!str `wrong_text` into configs.PartitionConfig"
}
```
+## Configuration
+
+Endpoint to retrieve the current scheduler configuration
+
+**URL** : `/ws/v1/config`
+
+**Method** : `GET`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```json
+partitions:
+- name: default
+ queues:
+ - name: root
+ parent: true
+ submitacl: '*'
+ placementrules:
+ - name: tag
+ create: true
+ value: namespace
+checksum:
+- 186
+- 179
+- 215
+- 100
+- 2
+- 130
+- 126
+- 171
+- 230
+- 47
+- 167
+- 228
+- 198
+- 188
+- 244
+- 216
+- 221
+- 149
+- 82
+- 131
+- 69
+- 97
+- 182
+- 182
+- 96
+- 239
+- 55
+- 254
+- 217
+- 41
+- 151
+- 145
+
+sha256 checksum: BAB3D76402827EABE62FA7E4C6BCF4D8DD9552834561B6B660EF37FED9299791
Review comment:
is this correct? Why there is a multi-line checksum?
##########
File path: docs/api/scheduler.md
##########
@@ -434,6 +434,76 @@ Reponse
}
```
+## Configuration update
+
+Endpoint to override scheduler configuration.
+
+**URL** : `/ws/v1/config`
+
+**Method** : `PUT`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```yaml
+partitions:
+ -
+ name: default
+ placementrules:
+ - name: tag
+ value: namespace
+ create: true
+ queues:
+ - name: root
+ submitacl: '*'
+ properties:
+ application.sort.policy: stateaware
+checksum: [173,7,83,162,71,12,24,54,61,72,195,227,207,95,85,14,128,101,107,7,112,214,125,208,136,131,212,118,44,191,90,222]
+```
+**Note:** the checksum is the base configuration checksum on top of what we want to make the changes.
+If the provisioned checksum differs from the actual scheduler checksum, the configuration update will fail.
+
+### Failure response
+
+The configuration update can fail due to different reasons such as:
+- invalid configuration,
+- outdated base checksum,
+- failure while saving the configuration to the scheduler.
Review comment:
When it fails to save the configs, that means something is invalid in the config, so it falls to the `invalid configuration`. I guess we can just remove this line, that might be less confusing if I am a user.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] kingamarton commented on pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
kingamarton commented on pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#issuecomment-708349121
> endpoint to retrieve the current conf
I will add some lines for getClusterConfig.
Related the refactoring of the validate-conf, that will be refactored only during the whole API refactoring, not? Does it make sense to do any kind of smaller refactoring before that?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] kingamarton commented on a change in pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
kingamarton commented on a change in pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#discussion_r505721685
##########
File path: docs/api/scheduler.md
##########
@@ -433,6 +433,141 @@ Reponse
"reason": "yaml: unmarshal errors:\n line 7: cannot unmarshal !!str `wrong_text` into configs.PartitionConfig"
}
```
+## Configuration
+
+Endpoint to retrieve the current scheduler configuration
+
+**URL** : `/ws/v1/config`
+
+**Method** : `GET`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```json
+partitions:
+- name: default
+ queues:
+ - name: root
+ parent: true
+ submitacl: '*'
+ placementrules:
+ - name: tag
+ create: true
+ value: namespace
+checksum:
+- 186
+- 179
+- 215
+- 100
+- 2
+- 130
+- 126
+- 171
+- 230
+- 47
+- 167
+- 228
+- 198
+- 188
+- 244
+- 216
+- 221
+- 149
+- 82
+- 131
+- 69
+- 97
+- 182
+- 182
+- 96
+- 239
+- 55
+- 254
+- 217
+- 41
+- 151
+- 145
+
+sha256 checksum: BAB3D76402827EABE62FA7E4C6BCF4D8DD9552834561B6B660EF37FED9299791
Review comment:
This is how the response looks like for getConfig()
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] yangwwei merged pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
yangwwei merged pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] kingamarton commented on a change in pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
kingamarton commented on a change in pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#discussion_r504615362
##########
File path: docs/api/scheduler.md
##########
@@ -434,6 +434,76 @@ Reponse
}
```
+## Configuration update
+
+Endpoint to override scheduler configuration.
+
+**URL** : `/ws/v1/config`
+
+**Method** : `PUT`
+
+**Auth required** : NO
+
+### Success response
+
+**Code** : `200 OK`
+
+**Content example**
+
+```yaml
+partitions:
+ -
+ name: default
+ placementrules:
+ - name: tag
+ value: namespace
+ create: true
+ queues:
+ - name: root
+ submitacl: '*'
+ properties:
+ application.sort.policy: stateaware
+checksum: [173,7,83,162,71,12,24,54,61,72,195,227,207,95,85,14,128,101,107,7,112,214,125,208,136,131,212,118,44,191,90,222]
+```
+**Note:** the checksum is the base configuration checksum on top of what we want to make the changes.
+If the provisioned checksum differs from the actual scheduler checksum, the configuration update will fail.
+
+### Failure response
+
+The configuration update can fail due to different reasons such as:
+- invalid configuration,
+- outdated base checksum,
+- failure while saving the configuration to the scheduler.
Review comment:
While saving the scheduler configuration it may fail due to some unexpected errors while updating the partitions, since if the validation passed, it should be OK in most of the cases, but in any case the response message will have the exact error message.
@yangwwei do you want me to remove this line, or how can it be formulated in a more explicit way?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-site] kingamarton edited a comment on pull request #29: [YUNIKORN-405] Document config update API
Posted by GitBox <gi...@apache.org>.
kingamarton edited a comment on pull request #29:
URL: https://github.com/apache/incubator-yunikorn-site/pull/29#issuecomment-708349121
> endpoint to retrieve the current conf
@yangwwei, I will add some lines for getClusterConfig.
Related the refactoring of the validate-conf, that will be refactored only during the whole API refactoring, not? Does it make sense to do any kind of smaller refactoring before that?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org