steveloughran commented on issue #1009: HADOOP-16383. Pass ITtlTimeProvider instance in initialize method in … URL: https://github.com/apache/hadoop/pull/1009#issuecomment-507263073 Review summary * IDE converted methods to single lines; they need to be restored to multiline entries < 80 char width. * the initialize(FileSystem, ITtlProvider) call could be replaced with just FileSystem given that we only support S3A FS for DDB, and it has a way to get that TTL provider, a way which is there, just being overwritten right now. The main arguments in favour of that explicit binding to a TTL provider are * pulls out the TTL provider as more important * nominally makes support for other filesystems easier (after all, LocalMS doesn't care, and we haven't yet changed the DynamoDB.initialize code to explicitly ask for an S3AFileSystem instance If you look at the slow refactoring I've started in HADOOP-15183, you can see that I've been pushing stuff into StoreContext, with the ultimate goal of all subsidiary parts of the s3a codebase (metastore, delegation tokens, S3ABlockOutputStream, etc) to not get given an S3AFS instance any more, just a StoreContext with access to lower level operations through binding callbacks (WriteOperationHelper, etc). Why so: it'll line us up for actually splitting up S3AFS into layers What does that mean now, well, not much: we can cast to S3AFS and extract the TTL, or take it as an argument as this patch does. I don't see it being that significant either way, except ultimately, I do hope to replace that initialize(FileSystem) with an Initialize(StoreContext), and, as that serves up the TTL, it'd be the consistent way to get it
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
