pranavsaxena-microsoft commented on PR #3440: URL: https://github.com/apache/hadoop/pull/3440#issuecomment-1375240277
> There, i've just spent a couple of hours going through it. big piece of work. > > In this current design, the EncryptionAdapter is either null or non null; if non null it is used to do the encryption/decryption, which is a bit scattered through the code > > There's another strategy: move the work into the EncryptionAdapter itself, with a an abstract EncryptionAdapter base class, a NoEncryptionAdapter for when its not used (make this a singleton) and then the ContextEncryptionAdapter which uses the EncryptionContextProvider, conains the keys etc and where you can push the work > > I'm worried that AbfsClient will call getPathStatus() on any operation when it things it needs the header, including getPathStatus itself. I think that code needs to be restricted only to those calls where it absolutely needs that header (do delete and flush really need it?), and that getPathStatus is explicitly excluded. > > Finally, is the new api live? In the method addEncryptionKeyRequestHeaders of AbfsClient.java, encrptionAdapter is always going to be non-null object. when encryptionType==ENCRYPTION_CONTEXT. Hence, we would not need to call getPathStatus in this method. And as there is no logic when encryptionAdapter is null, have not made the change for EncryptionAdapter base class and having NoEncryptionAdapter child class. -- 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]
