Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17974 )
Change subject: [encryption] KUDU-3331 Encrypt file system ...................................................................... Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/17974/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17974/7//COMMIT_MSG@45 PS7, Line 45: adds around 20% overhead > Cool, thanks! Done http://gerrit.cloudera.org:8080/#/c/17974/7//COMMIT_MSG@133 PS7, Line 133: > Yeah, I agree. It doesn't seem too useful to include. Done http://gerrit.cloudera.org:8080/#/c/17974/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17974/9//COMMIT_MSG@32 PS9, Line 32: ca > nit: 'be' --> 'is' or maybe add 'can'? Done http://gerrit.cloudera.org:8080/#/c/17974/9/src/kudu/tools/tool_action_pbc.cc File src/kudu/tools/tool_action_pbc.cc: http://gerrit.cloudera.org:8080/#/c/17974/9/src/kudu/tools/tool_action_pbc.cc@106 PS9, Line 106: IsFileEncrypted > nit: Is this actually meant to determine whether the file is sensitive or n I guess at this point it's to determine if it's sensitive, but if we switch to read the encryption header, it would be to determine if it's encrypted. Also, now I'm thinking it may not be too hacky, and don't really know if there's a clear benefit to check the encryption header. That way we need to actually open the file and read the first 8 bytes instead of just checking the filename. How would you feel about leaving this like this? -- To view, visit http://gerrit.cloudera.org:8080/17974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I909d0c4af0c1fca0d14c99a6627842dbe2ed7524 Gerrit-Change-Number: 17974 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 06 Jan 2022 19:51:34 +0000 Gerrit-HasComments: Yes
