[
https://issues.apache.org/jira/browse/HADOOP-15432?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16467974#comment-16467974
]
Sean Mackrory commented on HADOOP-15432:
----------------------------------------
Thanks for breaking this into smaller patches - it's much easier to review in
in smaller chunks that are more logically isolated. Is there an order to these
patches, or is the big patch just broken up into components? ABFS_SCHEME is
used but not defined in this patch, for instance, but it's defined in the
Constants patch. If there is an order in which they all apply nicely, please
share - I assumed this was first based on the JIRA numbers.
Haven't fully walked through all the functions in the main FileSystem
implementation yet, but it's mostly looking good. Some feedback & questions:
* Note that azure-bfs-auth-keys.xml (which isn't added by the way - although
that's probably intentional - just making sure you don't have a template that
you meant to add or something) is usually just called auth-keys.xml in the
other FSs (except for WASB). Don't have a strong opinion here, but if WASB will
eventually be dropped it might be nice to standardize on the auth-keys.xml
name. Just a suggestion.
* Sometimes you call the LoggingService LOG, other times it's
this.loggingService. I like LOG better - much more concise and consistent with
other code.
* I'm not much of an expert on HTrace or OkHTTP, but HTrace 4.x is already used
in Hadoop as is OkHTTP 3.7.0, but we're adding a dependency on HTrace 3.x and
OkHTTP 3.3.1. I'd like to make sure we have at least considered newer versions?
If there are valid reasons I don't object strongly, but for new code it's worth
some effort to not be locking ourselves into older dependencies already, IMO.
* The relocation pattern for shading Guava is a bit generic and could
forseeably conflict with some other app - I'd be more comfortable with
something like org.apache.hadoop.com.google... or something else that's clearly
specific to this usage.
* I understand suppressing ParameterNumber & FileLength in checkstyle - but the
others seem like they're easy to fix and I can't think of an obvious
justification. Can we fix the others in the machine-generated output before
committing it? If there's a strong reason to just use the machine-generated
output directly, I'd like some visibility into that process as we'll certainly
need to fix bugs in it from time to time.
* isSecure doesn't appear to get called anywhere. I suspect it's at least
supposed to be getting called when setting up abfsHttpService, maybe?
* There are like 4 different ways to specify a test account in the configs.
Other FS's do have distinct test FSs and contract test FSs, but 4 seems extra
redundant. Could you elaborate on why that is or consolidate them?
* Why aren't the root directory tests enabled?
* Is there a timeline for when people will be able to run the tests themselves?
> AzureBlobFS - Base package classes and configuration files
> ----------------------------------------------------------
>
> Key: HADOOP-15432
> URL: https://issues.apache.org/jira/browse/HADOOP-15432
> Project: Hadoop Common
> Issue Type: Sub-task
> Affects Versions: 3.2.0
> Reporter: Esfandiar Manii
> Assignee: Esfandiar Manii
> Priority: Major
> Attachments: HADOOP-15432-001.patch, HADOOP-15432-003.patch
>
>
> Patch contains:
> - AzureBlobFileSystem and SecureAzureBlobFileSystem classes which are the
> main interfaces Hadoop interacts with.
> - Updated Azure pom.xml with updated dependencies, updated parallel tests
> configurations and maven shader plugin.
> - Checkstyle suppression file. Since http layer is generated automatically by
> another libraries, it will not follow hadoop coding guidelines. Therefore a
> few rules for checkstyles have been disabled.
> - Added test configuration file template to be used by the consumers. Similar
> to wasb, all the configurations will go into this file.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
