fapifta edited a comment on pull request #2083: URL: https://github.com/apache/ozone/pull/2083#issuecomment-823660445
Hi @guihecheng, thank you for your dedication on working through this. I might misunderstand some parts of the suggestion. My false assumption was that the idea is to direct the query to a different method on the server side only, I did not expected a whole new proto request-response pair. My first impression was that this might be an overkill, thinking it through, it is cleaner to have a new request-response and handle it, than having a flag in a message, so I am fine with this approach as well, though I leave it up for others to decide if adding a new request-response into the proto layer is something we can do anytime, as I am not sure if it has any negative consequences to have many message pair for similar purposes. If it does not, and no one raises a concern, then the patch looks good to me, after the junit failure is fixed. I left a comment on the problematic line. I checked the integration test failures are also relevant, but it is not obvious for the first sight what is the problem. -- 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: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
