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 2021/04/13 09:33:17 UTC

[GitHub] [incubator-yunikorn-site] kingamarton opened a new pull request #47: [YUNIKORN-405] Extend documentation with checksum

kingamarton opened a new pull request #47:
URL: https://github.com/apache/incubator-yunikorn-site/pull/47


   


-- 
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 closed pull request #47: [YUNIKORN-405] Extend documentation with checksum

Posted by GitBox <gi...@apache.org>.
wilfred-s closed pull request #47:
URL: https://github.com/apache/incubator-yunikorn-site/pull/47


   


-- 
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 #47: [YUNIKORN-405] Extend documentation with checksum

Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #47:
URL: https://github.com/apache/incubator-yunikorn-site/pull/47#discussion_r612397448



##########
File path: docs/api/scheduler.md
##########
@@ -528,13 +528,20 @@ partitions:
         submitacl: '*'
         properties:
           application.sort.policy: stateaware
+checksum: BAB3D76402827EABE62FA7E4C6BCF4D8DD9552834561B6B660EF37FED9299791
 ```
+**Note:** the checksum is the base configuration checksum on top of what we want to make the changes.
+If the provided checksum in the request differs from the base version, the configuration update will fail.

Review comment:
       What is the base configuration? We need to be far clearer in what that is:
   ```
   Updates must use a current running configuration as the base. The base configuration is the configuration version that was retrieved earlier via a GET request and updated by the user. The update request must contain the checksum of the _base_ configuration. If the checksum provided in the update request differs from the currently running configuration checksum the update will be rejected.
   ```

##########
File path: docs/api/scheduler.md
##########
@@ -528,13 +528,20 @@ partitions:
         submitacl: '*'
         properties:
           application.sort.policy: stateaware
+checksum: BAB3D76402827EABE62FA7E4C6BCF4D8DD9552834561B6B660EF37FED9299791
 ```
+**Note:** the checksum is the base configuration checksum on top of what we want to make the changes.
+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.
 
 ### Failure response
 
-The configuration update can fail due to invalid configuration 
+The configuration update can fail due to different reasons such as:
+- invalid configuration,
+- outdated base checksum.
 
-In this case the transaction will be rolled back, and the proper 
+In each case the transaction will be rolled back, and the proper

Review comment:
       We do not roll back, the change is never applied. Better to say `rejected`

##########
File path: docs/api/scheduler.md
##########
@@ -528,13 +528,20 @@ partitions:
         submitacl: '*'
         properties:
           application.sort.policy: stateaware
+checksum: BAB3D76402827EABE62FA7E4C6BCF4D8DD9552834561B6B660EF37FED9299791
 ```
+**Note:** the checksum is the base configuration checksum on top of what we want to make the changes.
+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.
 
 ### Failure response
 
-The configuration update can fail due to invalid configuration 
+The configuration update can fail due to different reasons such as:
+- invalid configuration,
+- outdated base checksum.

Review comment:
       It is not an outdated checksum it is the incorrect checksum. We have no idea if it is outdated or just some random string.




-- 
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