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/05 15:08:13 UTC

[GitHub] [incubator-yunikorn-core] kingamarton edited a comment on pull request #207: [YUNIKORN-405] Added checksum validation

kingamarton edited a comment on pull request #207:
URL: https://github.com/apache/incubator-yunikorn-core/pull/207#issuecomment-703694183


   > I just added a minor comment. Overall the PR looks good, it has good UT coverage, which is good.
   > The concern I have is not for the changes itself, but the workflow when a client starts to consume the API.
   > If the checksum failed, updateConfig fails, the client-side could be very confusing... what do we expect them to do? Is it supposed to ask them to call a GET to reload the latest config from YK, and overwrite their existing configs in whatever storage? What if users already made some changes in their local, how they can restore that, and apply to the latest content? This is very complex.
   
   @yangwwei  yes, in case of stale config the client will bee notified that they should update their configuration before applying their changes. If we will not add the checksum validation, we might be in the case that the user is performing some unwanted changes, like deleting some changes made by an another user, without even noticing it. @wilfred-s  any thoughts on 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