|
Wouldn't it make more sense to keep the FileSystem interface for clients and evolve it, and move the implementor's interface to an SPI?
Also, instead of creating a new interface, we might be able to use the new FileSystem interface being developed in JSR 203. See HADOOP-3518. It might be nice to make it simpler to open a file given a URI and assuming the default configuration, but there are hazards to that as well. Application code often becomes library code, and changing code that does not explicitly pass a configuration to start passing a configuration breaks compatibility. The Hadoop API convention is everything must be explicitly passed a configuration, so that we do not rely on static state. This is just a cost of doing business, and it's dangerous to remove it.
So I agree that static utility methods would be nice, since the FileSystem implementation can be automatically inferred from the configuration and the URI, but I think the Configuration should still be explicitly passed to these methods. If we don't think that applications should ever need to call the non-static FileSystem methods, then placing the static methods in a different class makes sense, since it consolidates the documentation that users need be aware of. Does that make sense to you, Tom? Also, create() has too many parameters and is fragile. We should add a FileCreation class that encapsulates these. We should consider adding such classes for any methods that take more than a Configuration and a Path. Doug do you mean you would prefer something like:
Files.create(config, path, ...) File.open(config, path, ...) The exportConfig and importConfig methods in the proposed patch (Owen's suggestion) are for the library writer, but the library writer has to be aware of it. > Also, create() has too many parameters and is fragile. The proposed Files interface significantly reduces the number of create methods. It accomplishes this partly by Thus the first create method is very simple and robust for type checking: public static FSDataOutputStream create(Path f, FsPermission permission, boolean overwrite) throws IOException The second create methods is fragile since many of the parameters are ints and one can easily mix the parameters. Given that most app writers will use the above simpler method, I am not as worried as I used to be. Were you thinking of something like: // Style 1 createdFile = create(path, permission, overwrite) createFile.setBlockSize(512); // optional most are okay with defaults outputStream = createFile.open(); // Style 2 outputStream = create(path, permission, overwrite); // the common case where the defaults are fine createParms.setBlockSize(512); // for when the defaults are not fine. outputStream = create(path, permission, overwrite, createParms); // For comparison, the proposed patch would have required the following outputStream = create(path, permissions, override); // common case when defaults are fine outputStream = create(path, permissions, override, 512, -1, -1, progress); // when you want to override the defaults > Doug do you mean you would prefer something like: Files.create(config, path, ...)
Yes. We require a configuration in nearly every other API so I don't think eliminating one here actually saves much and it makes it likely that folks will write code that mistakenly assumes the default configuration. As for create options we might try something like: outputStream = File.create(config, path, new FsCreate().permission(p).blocksize(b)); We might also provide a version like: outputStream = File.create(config, path); that is equivalent to: outputStream = File.create(config, path, new FsCreate()); Doug> Application code often becomes library code, and changing code that does not explicitly pass a configuration to start passing a configuration breaks compatibility.
I guess I am missing this library argument and having a hard time buying the argument "the rest of the Hodoop APIs do this and so we need to do this here". Most systems do not have such a config parameter in their APIs and they support both Application code and library code. What is so different about Hadoop and its libraries? Some example of the libraries you have in mind would help. Most file system interfaces are dead simple. I think Hadoop's API can be made equally simple and at the same time allow complex libraries to be built relatively easily. I thought the exportConfig/importConfig facilitates that. Your use-case is where a piece of application code suddenly turns into library code. I think there is more to the underlying environment than just the config; hence libraries have to be more careful . Owen, Arun and I discussed the Task Tracker and whether or not it can do any work on behalf of the job (either a some pre-work (loading cache) or the task itself. The task tracker has to take the credentials and the class path from the job submitter's environment. For example when the task tracker acts on behalf of the job submitter, it has to do the equivalent of JAAS's callerSubject.doAs(). > I guess I am missing this library argument [ ... ]
What I mean is that there isn't a clear line between application and library code, that folks might wish to be able to, e.g., start running jobs on two clusters at once, without having to refactor everything to keep track of the config. We should not encourage a style that makes this kind of thing more difficult, but rather one that makes it easy. > Most file system interfaces are dead simple. I think Hadoop's API can be made equally simple [ ... ] I agree. But I also think we need to pass the configuration explicitly, and not depend on a static configuration. Our current convention is that only command-line tools create all-new configurations and I think there are good reasons to stick with that. > In most file systems, the config state is provided by the underlying OS; The configuration is the equivalent of the unix environment. Using per-process environment variables makes sense when you start new processes for new operations, the unix convention, but that's not the way folks generally work in Java. We could use a static configuration, in fact we used to long ago, but that caused lots of problems. In other projects I've frequently seen folks start out using Java system properties, then switch to something that's passed dynamically. That sort of switch is painful and something we should avoid and encourage others to avoid. > In my proposal the libray writer will have to add a line "Files.importConfig(configArg)". That does not work for multi-threaded applications, where different threads are configured differently. Consider, e.g., a Nutch-based system that has different search zones (like, e.g. http://www.archive-it.org/ Thanks for the clarifications; I understand the issue better.
>> In my proposal the libray writer will have to add a line "Files.importConfig(configArg)". Depends on how you implement it. Thread local variables (as we do for the UGI credentials) would address MT.. We have at least the following 3 options 1) Class with static methods (as per patch) using thread local variables to store the 2) Class with static methods and a config parameter for each method 3)Class with instance methods Class Files { static Files defaultConf(); Files(config) //open, create etc are all instance methods } myFiles = Files.defaultConf(); // common usage myFiles = new Files(myConf); // for those that want a specific conf myFiles.create(...) I have taken an initial look at the new NIO2 comping in jdk7 (JSR 203 - http://openjdk.java.net/projects/nio/
NIO2 has notions of multiple file system providers (like hadoop) and also has a URI based file system. Technical considerations for adopting JSR 203:
There are non-technical issues such as the timing of NIO2 and jdk7; Alan Bateman, In NIO2 the default file system (ie slash relative names like /foo/bar) are relative to the JVM's file system. However, Hadoop has a slightly different notion of default file system. Hadoop file names are as follows
Pathnames of type (3) or (4) are not consistent with NIO2 and the rest of Java io. Consider alternate name conventions like hfs:/foo/bar and hfs:foo/bar as Hadoop default root and hadoop working directory: IMHO, these will be too inconvenient: e.g. at the shell, instead of typing "ls foo/bar" one would have to type ls hfs:foo/bar" to list files relative to the working directory. I don't see how we can escape this dual notion of root and wd: Path p = Paths.get("/foo/bar"); // relative to jvm default fs. Path p = HDFS_provider.get("/foo/bar"); // relative to hadoop's defautl fs. Path q = Paths.get("foo/bar"); // relative to jvm's default fs wd. Path q = HDFS_provider.get("foo/bar"); // relative to hadoop's wd. The other thing to evaluate is the file system provider interface. Please continue this discussion on HADOOP-3518. If JSR 203 provider interfaces are not Also wrt timing we will have to move forward with our APIs for hadoop 1.0 I do not fully grok the problem here. Is the meaning of a working directory different in HDFS from that in the local file system?
I've just briefly browsed through the NIO.2 FS API and, as far as I can tell, the notion of working directory doesn't really exist. It is only applicable to file systems where the environment (working directory, or similar) is implicitly available (or if there is none). In my opinion, this is a good thing. In this respect: Paths.get("hdfs://...") doesn't make much sense because the environment for HDFS, the Configuration object, is not available nor is a FileSystem reference. Wrt. 3. I'm not sure I understand the meaning of "slash relative names"; isn't that an absolute path? > Is the meaning of a working directory different in HDFS from that in the local file system?
Yes. The local filesystem's working directory is global to the JVM and read-only, while Hadoop FileSystem instances each have a working directory that may be changed. For LocalFileSystem, the working directory is initially the JVM's working directory. For HDFS, the working directory is initially the user's home directory in that filesystem (/user/${username}). Relative paths are resolved against the working directory. So, "foo" refers to a file in the working directory of the default filesystem while "file:foo" refers to a file in the working directory of the local filesystem. The default filesystem is determined from the Configuration in effect. In the NIO.2 API there is no concept of a working directory. But, paths aren't resolved against the working directory, but rather against any other directory, so it's easy to introduce a working directory concept on the application level.
> In the NIO.2 API there is no concept of a working directory.
Rather I suspect the working directory concept in NIO.2 is specific to the URI scheme, as in Hadoop, that for file: uri's the java processes working directory would apply, so that file:foo A material difference (that Sanjay alluded to above) is that the default filesystem (uri scheme & authority) in NIO.2 is fixed to be the local filesystem (file: Yes. I'm not saying that the API defined in JSR 203 maps onto HDFS perfectly.
So, after proposing this as a GSOC in https://issues.apache.org/jira/browse/HADOOP-3518 Hopefully, you guys think it's a good idea, too. I'll be very glad for any suggestions or ideas. (The application deadline has passed, so the application can't be changed. But all other feedback is appreciated.) The attached Files.java is a class for the proposed new interface for the application writer. The attached FilesContext.java is a class for the proposed new interface for the application writer. (I have use the name FIlesContext instead of the original class name Files since the class/object is really a context in which files
are named and created; further it avoids confusion with the Java class File. The FileSystem class will remain for the fs implementing FSs (after some cleanup); We will probably rename it to AbstractFileSystem or VirtualFileSystem (since it is very similar to the Unix VFS). The FilesContext class is thing layer that mostly calls FileSystem. There are still a few methods that remain to be implemented (such as copy, move, glob). Special things to note when reviewing the new interface
Some naming nits:
Updated patch.
(Patch is not complete yet - has missing parts and some bugs I know of). Addressed Doug's naming nits + some bug fixes. > * Why do we need the inner class FSConfig? Its fields seem like they should be FileContext fields directly, no? > Kept it as a separate class because it has nothing to do with FilesContext; it merely sets/gets
> keys from the Config. It sets/gets the keys that FileContext uses, no? I suppose others may read them, but, if most file access is through FileContext, then FileContext has a primary association with these keys. > Besides other parts of fs may need to access the keys or their default They're static – other parts of the FS can access them. I still don't see why these static methods belong on a separate class when they're so closely tied to FileContext. Doug, I would like to keep them in a separate class:
The proposed FileContext API, in addition to being a more convenient interface for application writer, will also simplify the FileSystem layer, freeing that layer from dealing with notions of default filesystem (i.e. /) and wd and also freeing it from having to deal with default values of FS variables like default block size (BS), replication factor (RF), umask, etc
The original design was to allow options 1 and 2 below for config values. A few folks felt that we needed to retain the notion of client-side defaults (option 3 below).:
After further thinking, I am proposing that we support only options 1 and 2; not 3.
I forgot to mention that the class FsConfig is suppose to be package private (I had accidental made it public).
This should clear up the confusion between Doug and me. A couple of initial comments:
> * There is a FileContext in the metrics package too, so perhaps we should rename this.
Hmm. didn't know that. FileContext is a very appropriate term since it is context for resolving file names. The term context is used heavily in naming literature for that purpose. In our discussion, folks got the whole concept of what I was trying to do as soon as I changed the class name from Files to FileContext. I am tempted to leave it as is since it is in a different package. I don't want to call it a FSContext since it s not a context for FileSystems. Strictly specking it is context for file names. Perhaps I should call it PathContext or FileNameContext. Or perhaps rename the metrics one since it is an internal class.? Do you have preference in one of the options I suggested? Or another suggestion. > * What's the util() method for? Why are its methods not first-class methods of the interface. Seems confusing to users. No one has commented on my proposal on the config issue in this jira. As a result, over the last 2 days, I have had a set of discussions with a number of folks at Yahoo, including Doug and with Dhruba. Here is roughly the set of opinions:
Based on the feedback, a proposal is described below. Note for some folks parts of this proposal represents a compromise, but they could live with it. The 21 deadline is very very close and we need to get this in or we will miss the deadline. FileContext contains the following items derived from the config:
One creates FileContext as described in the patch (the patch is not uptodate with the proposal in this comment).
NO other config parameters are read from the config: The fs client side config contains only two things: your / and your umask; all defaults will come from SS. However, users will be able to override these defaults through the options parameter in the create() call when creating a file. So in this proposal there is not way to set application defaults in the config file. So the basic calls are:
Examples of create using varargs Roughly: CreateOpt is a class with several subclasses, one per option (Blocksize, RepFactor etc) and a static factory method for each of them such as CreateOpt.blocksize(long). Here is the list of options that one will be able to set through the createOptions:
The following SS variable is not settable via the createOption.
If the application desires a particular property it will set it in the createOpt paramaters. There is no automatic support to read these app defaults from a config file; this was deliberate choice. The actual mechanisms for createOpts is still to be determined but I am strongly leaning towards varargs rather then a options-Object with setters and getters. So please comment on this proposal ASAP. The above proposal was derived after looking at several alternative and lots of discussions; thanks to all those who participated. ------ +1 for this proposal. I vote that we should remove the Progressable option to the create() call.
> Shall we remove progressable?
FWIW, this was added in My earlier assumption was that the DSFSClient was not even updating Progress. But looking at the code, I find that the DFSOutputStream.DataStrteamer (that streams packets to datanode(s) in the pipeline) is actually updating the "progress" status after packet sends. In that case, we should keep the Progress parameter in the create call.
+1 on CreateOpt + VarArg
Why perms is not an instance of CreateOpt? I also dislike operations that return false when there is a failure; I prefer an exception.
But in some cases the choice is not not obvious: mkdir(path); One can argue that if the path does not exist it should returns true. Same for delete. From what I understand, Hadoop has followed the java file apis in a few places. (correct?) — I looked through FileContext9.patch in detail last night. My notes are below. Obviously this is still a preliminary patch, but I wasn't sure I'd have time to look again for a few weeks, so figured I'd give a detailed review now - feel free to ignore notes about things that are preliminary bits and won't be in the final patch.
FileContext.java;
code:
overall:
FsConfig.java: nits:
code:
FileSystem.java:
FileContextBaseTest.java:
+1. I always assumed CreateOpts object would be created and then passed as single argument. +1 for chaining pattern to set arguments.
This was also my reaction, but was persuaded that the inevitable component that passed CreateOptions objects around would be more confusing than forcing the params into the caller. Of course, nothing prevents that hypothetical component from building and passing an array of CreateOpts (or an ArrayList<CreateOpts>) through a tangle of functions and ultimately to create, but at least the API is clear. Given that the latter is available for anyone convinced that they need to pass a set of CreateOpts though several layers, the single-object CreateOpts seems to offer few advantages other than keystrokes and the impossibility of duplicate entries in the varargs. (Todd: for readability, would you mind trying to compose feedback in a less verbose format? Noting that spelling errors, indentation, and grammar need cleanup in a preliminary patch need not take a dozen bullets; critiques of specific variable names drown out more important points and can be generalized)
My issue is that the multi-argument form pushes the handling of defaults down into the FS implementation, whereas it was my understanding from this patch that the defaults should be handled by FileContext (or another FS-independent class). If there is a single-object CreateOpts, it could either have the defaults preset by its constructor, or have getters that returned defaults when the variables had not been set.
Yes, I apologize for having such a lengthy/detail-oriented review of a preliminary patch. Given the 0.21 freeze is coming up soon, and my next two weeks are pretty busy, I wanted to get all my comments out there now in case I didn't have time to look again. For future reviews I'll try to more cleanly delineate the detail-oriented things from more important macro issues. A proper code review tool hooked up to JIRA would be invaluable here.
Since the defaults are conceptually server-side, I'm assuming that that a FileContext 1) may not know them and 2) won't keep its own set of defaults as state. The intent, I think, is to let these be configurable by passing them through FileContext and into the implementation, which may elect to disregard some options (e.g. replication on local). None of the parameters for create really need getters; it would be an odd client that needs some ratio of the cluster's default block size, rather than a specific value coherent for the caller. Fair number of updates and cleanup since friday (is this why it is called Labor day weekend?)
Glanced over Todd's comments - turns out i had already fixed many. I will response to/fix his feedback in the next couple of days. Note 2 matching patches for common and hdfs. I don't feel that strongly about the CreateOpts thing. It might result in a lot of duplicate code across FS implementations (the loop-through-and-populate-variables pattern) but if people think it's clearer than a single object, +1.
@todd
>rather than FileSystem.getInitialWorkingDirectory() returning null by default and having the check in FileContext, have it default to just passing through to getHomeDirectory() in FileSystem.java I went back and forth on this one. Decided that the filesytem layer needs to be dumb and return information to upper layer (FileContext) and let it set defaults as needed >could makeAbsolute be made public? It seems generally useful >are these kinds of paths [relative paths with schemes] ever legal in Hadoop? If not, can this check go into the Path constructor such that we can never end up with an invalid object? > in favor of removing this constructor entirely and forcing the user to explicitly choose to construct a new Configuration(). > Why allow the user to pass either URI or FileSystem instances? There's less code if you just provide one, and the user can always go from one to the other. I'm in favor of fewer code paths where possible. > A lot of the stuff in util is copied straight from FileSystem.java. This code duplication should be avoided. @InterfaceStability.Evolving /* soon to be stable after release */ This should be marked as Stable in the patch then, no? Folks should only use releases, not trunk, and we don't want to have to change all of the new APIs from Evolving to Stable when we branch for a release. In other words, its okay to incompatibly change an API that's marked Stable if it's never been released. Does that make sense? On the other hand, since this is a new API, it might make sense to leave it as Evolving for a release, so that we have a chance to improve it in case we find problems with it. @doug.
My reason was: mark it evolving for a release in case we have changes (i.e your "on the other hand"). The 1.0 rules do not apply right now; with the current hadoop rules everything is evolving. Minor changes in the patch.
All/most of my patches have used 2 new protected methods to support FileContext. From my understanding, the objections raised in Rename jira would apply to these. Hence we are exploring a significantly different implementation strategy. I request the community to continue review the FileContext api in this There have been two comments (tom and sanjay) on methods that return boolean. (mkdir and delete).
If you care about this issue please add your opinion. Propose that we change create and mkdirs apis as follows.
Create and mkdir are atomic; the recursive versions are not guaranteed to be atomic. (separate discussion on whether or not There is a jira (my search attempts failed to find it) that has argued that a non-recursive create is important for MR. IMHO the recursive create and recursive mkdirs of FileSystem were a big mistake; we should have stuck to the Unix spec; every diversion from > My reason was: mark it evolving for a release in case we have changes (i.e your "on the other hand").
That's fine. I was confused by the comment. Let's change that to say "for a release" then. Glancing through the patch, lines 2071 and 2072, one of those calls should be using dst, rather than src while checking scheme
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12419722/FileContext-common15.patch against trunk revision 815524. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 70 new or modified tests. -1 javadoc. The javadoc tool appears to have generated 1 warning messages. -1 javac. The applied patch generated 171 javac compiler warnings (more than the trunk's current 145 warnings). -1 findbugs. The patch appears to introduce 1 new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/37/testReport/ This message is automatically generated. -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12419751/FileContext-common18.patch against trunk revision 815524. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 8 new or modified tests. -1 javadoc. The javadoc tool appears to have generated 1 warning messages. -1 javac. The applied patch generated 171 javac compiler warnings (more than the trunk's current 145 warnings). +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/38/testReport/ This message is automatically generated. -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12419752/FileContext-common18.patch against trunk revision 815524. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 8 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 171 javac compiler warnings (more than the trunk's current 145 warnings). +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/5/testReport/ This message is automatically generated. -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12419781/FileContext-common19.patch against trunk revision 815809. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 8 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 171 javac compiler warnings (more than the trunk's current 145 warnings). +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/39/testReport/ This message is automatically generated. +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12419803/FileContext-common21.patch against trunk revision 815809. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 8 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/40/testReport/ This message is automatically generated. minor comments and spacing changes from previous version.
Some minor comments:
These are minor comments. This can be changed when additional changes are going to be done for new rename functionality. +1 for the patch. Ok, I finally got around to looking at this jira...
Given the discussion around the 2 options in
+1 Overall, though. This is a reasonable path to the new API. The latest patch looks good. A few minor comments and questions:
There are some places where the "Deprecated" tag is not in place because it generates HudsonQA warnings. How about if we tag the appropriate methods as Deprecated and then knowingly ignore the "-1 from HudsonQA for javac warnings"? > How about if we tag the appropriate methods as Deprecated and then knowingly ignore the "-1 from HudsonQA for javac warnings"?
Alternately one can use @suppressWarnings("deprecation") to silence these if the uses are few. On absolute paths etc:
>would applications need to access FileContext.makeAbsolute()? Can this method be made public? Okay here is how I have been using the terms. It seems to agree with what is in Path(). Applications often want to convert a path to fully qualified if they want to pass it around or store it in a file. So we don't need to expose FileContext#makeAbsolute () right now- we can add it later if there is a use case. (BTW my FileContext#makeAbsolute() was bad method name - its comment and impl states that it merely fixes the relative part. > Path.isAbsolute() and Path.isPathComponentAlbolute() look strikingly same to me. do we need both? I had intended to file a Jira to explore this and fix it if necessary. If it turns out that all we need is isPathComponentAbsolute()
Can we document these in the javadoc for Path? I've always found it confusing and I think having a clear definition in the code will be very helpful moving forward. Douglas > FileContext::getDefaultFileSystem is temporary?
It is for tests. Changed it to protected and added stability tags. > Can FileContext::isFile and FileContext::isDirectory be removed? These are redundant with getFileStatus These are convenience methods. Old API supports this and is used extensively. > The primitiveCreate(), primitiveMkdir() methods seem to have there only for backward compatibility and would probably go away in a future release. Can it be marked as Deprecated?
These are newly added protected methods to support FileContext on top of FileSystem (nothing to do with backward compatibility). Thanks for the comments, sanjay. Looks good to me. +1
Patch addresses the issues raised.
Path#makeQualified(FileSystem f) is now deprecated. We will work with hudson to overlook the warnings (as Dhruba suggested). Will also file a jira to fix the code that calls this deprecated method. > These are newly added protected methods to support FileContext on top of FileSystem (nothing to do with backward compatibility).
Still, marking them as deprecated seems wise, since we don't want folks to start calling them, do we? +1, we seem to have consensus about the approach to get this in.
@doug > Still, marking them as deprecated seems wise ...
Done. New patch uploaded. -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12419927/FileContext-common25.patch against trunk revision 815809. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 8 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 174 javac compiler warnings (more than the trunk's current 145 warnings). +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/41/testReport/ This message is automatically generated. +1 since there is a plan in place to address the javac issues. Normally it would be good to address them in the same patch that generates them, but in the interest of keeping this patch as concise as possible this is a fine solution in the case of deprecating a method.
I just committed the change. Thank you Sanjay.
Integrated in Hadoop-Common-trunk-Commit #35 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/35/
. Add new improved file system interface FileContext for the application writer. Contributed by Sanjay Radia. Integrated in Hadoop-Common-trunk #101 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/101/
. Add new improved file system interface FileContext for the application writer. Contributed by Sanjay Radia. Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #35 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/35/
Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #11 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/11/
Editorial pass over all release notes prior to publication of 0.21.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
The FileSystem class will remain for the fs implementor (after some cleanup); it will also be used to implement the
Files class.
None of the methods in the Files class have an implementation yet - the skeleton is to discuss the proposal.