[
https://issues.apache.org/jira/browse/HDFS-11170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15988536#comment-15988536
]
Steve Loughran commented on HDFS-11170:
---------------------------------------
When people make changes to the FileSystem class, can they
# update what they actually do in the Filesystem specification markdown files,
so we have a full declaration of what's meant to happen. Yes, it's extra work,
but it forces you to think through what cross-FS implementation expectations
are, rather than just have them do whatever HDFS does
# write contract tests so that the downstream filesystems can all share
# and when we do a new operation, we get to revisit what is wrong with the
current one, rather than just repeat the old behavior.
One way to to assist this is to just warn me that this is going on, and while I
won't do the work, I can help.
Now some comments on the code of various levels of importance
* Why is FileContext left out?
* {{newFSDataOutputStreamBuilder()}} isn't a good name. It describers the
return value, but not what it is trying to do, which is `createFile()`
* If you look at what is wrong with {{FileSystem.create();}}, the fact that
callers can expect a directory to be created is a problem; it's very expensive
on object stores where we have to walk the tree and look for things above
(files). If you look at the codepaths of how create() is used, most people
create the parent dir anyway, they don't relay on this feature. So we could
make it yet another builder option ("createParentDirs") and have people
explkicitly create it if they want, leave it off by default (yes, we'll have to
tweak filesystem.create somehow, but we can do that across the board in our own
code)
& on the tests
* I don't think the HDFS test case needs to bring up a test cluster just for
one test suite; it's slow. Again, Moving this into a new Contract test and then
having the HDFS be one of the concrete implementations will sort this out. If
added to {{AbstractContractCreateTest}} then all filesystems will get this test
added for free, which is what is required if they are all expected to support
the API.
* your asserts all get their expected/actual values wrong. Flip the order
I'm going to create a new JIRA to finish up the issues,. I really do want this
to be stable. At the very least, release notes must indicate the fact that this
is still stabilising,
Sorry for getting in so late & causing trouble, but I'd only just noticed that
a change had happened to FileSystem.java after the fact.
> Add builder-based create API to FileSystem
> ------------------------------------------
>
> Key: HDFS-11170
> URL: https://issues.apache.org/jira/browse/HDFS-11170
> Project: Hadoop HDFS
> Issue Type: Improvement
> Reporter: SammiChen
> Assignee: SammiChen
> Labels: hdfs-ec-3.0-nice-to-have
> Fix For: 2.9.0, 3.0.0-alpha3
>
> Attachments: HDFS-11170-00.patch, HDFS-11170-01.patch,
> HDFS-11170-02.patch, HDFS-11170-03.patch, HDFS-11170-04.patch,
> HDFS-11170-05.patch, HDFS-11170-06.patch, HDFS-11170-07.patch,
> HDFS-11170-08.patch, HDFS-11170-branch-2.001.patch
>
>
> FileSystem class supports multiple create functions to help user create file.
> Some create functions has many parameters, it's hard for user to exactly
> remember these parameters and their orders. This task is to add builder
> based create functions to help user more easily create file.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
