You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Sean Mackrory (JIRA)" <ji...@apache.org> on 2018/05/08 21:15:00 UTC
[jira] [Commented] (HADOOP-15432) AzureBlobFS - Base package
classes and configuration files
[ 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