hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sean Mackrory (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-15432) AzureBlobFS - Base package classes and configuration files
Date Tue, 08 May 2018 21:15:00 GMT

    [ 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: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Mime
View raw message