skysiders commented on PR #3894: URL: https://github.com/apache/hive/pull/3894#issuecomment-1387202288
Hi @zabetak , thanks for your review. You mentioned that this is "kind of a breaking change", but I don't agree with this. In setting file permissions, most of the time we can rely on the umask of the underlying file system, such as the most commonly used fs.create function, but in In my fix, all file permissions are explicitly set. I think this explicit permission setting is due to the developer thinking that the file should be set to this explicit permission. If the development of this pair of files does not require explicit permissions, the underlying umask can indeed be used to constrain it, but once the permissions are clear, the underlying umask may cause the file permissions to be too strict and make the files unusable. I would like to give an inappropriate example here. For example, the umask of the underlying file system is 777, and the file permissions are 000, so the upper-level files will not have any permissions. Therefore, for such files with clearly set permissions, I think it should be Make sure they a re properly assigned permissions. Regarding the second point "programming pattern" you mentioned, in fact, it is also possible to use Hadoop's underlying FileSystem.create(fs, path, perm) here. In fact, I now think that such a "programming pattern" should be adopted, because this It is safe and more reliable than fs.create(path,perm). This kind of repair is mainly aimed at API misuse. I have mentioned this problem in[ HBASE-26994](https://github.com/apache/hbase/pull/4391), which means that the developer originally intended to Set special permissions here, but mistakenly think that fs.create(path, perm) can set special permissions perm for the path. In fact, this is wrong. In the chat with the hbase developer, I pointed out his mistake and Got his approval. Finally, I want to say that this fix is necessary in my opinion. I have searched in hive and found these four API misuse problems, so I point out this problem here. -- 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]
