skysiders commented on PR #3894: URL: https://github.com/apache/hive/pull/3894#issuecomment-1372314783
Hi @cnauroth ,thank for your review! What I want to talk about here is a programming mode, which is implemented for special permissions. For example, we should check this permission after setting the special permission. You are right, the 700 permissions here will not cause defects with a high probability, but I think a good programming style should be maintained to ensure the correct granting of file permissions, so here I first judge the file permissions and check the file permissions Whether it is correctly granted, if not, then set the permissions for them. However, if the permissions here are changed to 770 or other permissions that may be affected by the umask in the future, there will be defects. Therefore, from a rigorous point of view, I think it is necessary to further check the permissions here. I have mentioned similar flaws in [STORM-3862](https://github.com/apache/storm/pull/3478) ,[TEZ-4412](https://github.com/apache/tez/pull/209),[HBASE-26994](https://github.com/apache/hbase/pull/4391) Finally, regarding the FileSystem.mkdirs(fs, path, perm) you mentioned, I agree with your point of view, but what I still want to say is that the idea of this kind of repair is not to set permissions on files, but out of file permissions Check, as shown in the code, if the permissions are correct, we don't need to set permissions on it again. -- 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]
