dombizita commented on PR #4478: URL: https://github.com/apache/ozone/pull/4478#issuecomment-1496357296
just summarise the discussion a bit @sumitagrawl. so to be honest I have cons for both of the approaches. with my approach I don't like that previously there were no relevant checks on the client side (in the `BasicRootedOzoneFilesystem`s `renameFSO()` method) for the FSO buckets, only on the OM side in the `OMKeyRenameRequestWithFSO` class. also with my current changes I just handle if the destination is a bucket and then add the source key name, in case of a directory destination path I just pass it on (so it is not consistent) as it will be a valid path and the source key name will be added in the OM side. in case we do changes in the `OMKeyRenameRequestWithFSO` we need more changes that I wanted to avoid. first we need to either add a `preExecute()` method for the FSO implementation or remove/move the `validateAndNormalizeKey` to somewhere later in the code. second we need to remove the `toKeyName.length() == 0` check from the `validateAndUpdateCache()` method, but there are a few places where we use the empty `toKeyName` before we handle if the destination path is a directory and add the source key name. so there needs to be done a few changes which can affect other behaviours. as I looked into this more, your solution would be more consistent with the current code, but I think that these kind of checks should have happen consistently at the same place in both bucket types, as I think nothing explains why we do it differently. but that is already the way it is :) -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
