[
https://issues.apache.org/jira/browse/HADOOP-18987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17789115#comment-17789115
]
ASF GitHub Bot commented on HADOOP-18987:
-----------------------------------------
steveloughran commented on code in PR #6292:
URL: https://github.com/apache/hadoop/pull/6292#discussion_r1403319403
##########
hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md:
##########
@@ -432,7 +432,7 @@ of data which must be collected in a single RPC call.
#### Preconditions
- exists(FS, path) else raise FileNotFoundException
+ if not exists(FS, path) : raise FileNotFoundException
Review Comment:
I was using the concept exists() holds or raise FNFE, but if this suits then
I'm not going to argue about details. The goal of this spec is for other people
and tests, not mathematical purism.
##########
hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md:
##########
@@ -88,7 +88,7 @@ Get the status of a path
stat.length = len(FS.Files[p])
stat.isdir = False
stat.blockSize > 0
- elif isDir(FS, p) :
+ elif isDirectory(FS, p) :
Review Comment:
`isDir(FS, Path)` is the function in the model; 'isDirectory()` path the
published method in Filesystem.
##########
hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md:
##########
@@ -1395,31 +1397,31 @@ then `overwrite` flag must be set to TRUE.
#### Preconditions
Source and destination must be different
-```python
+```
Review Comment:
go on, reinstate these. for the sake of intellij's code colouring
##########
hadoop-common-project/hadoop-common/src/site/markdown/filesystem/model.md:
##########
@@ -215,9 +219,9 @@ This may differ from the local user account name.
A path cannot refer to more than one of a file, a directory or a symbolic link
- FS.Directories ^ keys(data(FS)) == {}
- FS.Directories ^ symlinks(FS) == {}
- keys(data(FS))(FS) ^ symlinks(FS) == {}
+ directories(FS) ^ filenames(FS) == {}
Review Comment:
probably the opportunity to delete the extra space in front of the ^ symbols.
##########
hadoop-common-project/hadoop-common/src/site/markdown/filesystem/abortable.md:
##########
@@ -87,15 +87,14 @@ for example. output streams returned by the S3A FileSystem.
The stream MUST implement `Abortable` and `StreamCapabilities`.
-```python
- if unsupported:
+```
Review Comment:
can you restore this. yes, it bears more relation to the Z specification
language of Ince93 etc, but if you tag as Python then IDEs cover it. And Github
doesn't really handle any of the formal languages. Nor do most developers,
which is why this uses a python-esque syntax.
##########
hadoop-common-project/hadoop-common/src/site/markdown/filesystem/model.md:
##########
@@ -108,21 +108,21 @@ such as `rename`.
## Defining the Filesystem
-A filesystem `FS` contains a set of directories, a dictionary of paths and a
dictionary of symbolic links
+A filesystem `FS` contains a directories (a set of paths), files (a mapping of
to bytes) and symlinks (a set of paths mapping to paths)
Review Comment:
* cut the "a" before directories
* files (a mapping of path to lists of bytes)
##########
hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md:
##########
@@ -826,9 +826,9 @@ any successors.
#### Postconditions
- result = FSDataInputStream(0, FS.Files[p])
+ result = FSDataInputStream(0, FS'.Files[p])
-The result provides access to the byte array defined by `FS.Files[p]`; whether
that
+The result provides access to the byte array defined by `FS'.Files[p]`;
whether that
Review Comment:
open() is not mutating; FS` = FS; therefore FS is all that is needed
##########
hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md:
##########
@@ -1319,28 +1320,30 @@ Implementations without a compliant call SHOULD throw
`UnsupportedOperationExcep
All sources MUST be in the same directory:
- for s in sources: if parent(S) != parent(p) raise IllegalArgumentException
+ for s in sources:
+ if parent(s) != parent(p) : raise IllegalArgumentException
All block sizes must match that of the target:
- for s in sources: getBlockSize(FS, S) == getBlockSize(FS, p)
+ for s in sources: getBlockSize(FS, s) == getBlockSize(FS, p)
Review Comment:
should this line be split for consistency with the precondition above?
> Corrections to Hadoop FileSystem API Definition
> -----------------------------------------------
>
> Key: HADOOP-18987
> URL: https://issues.apache.org/jira/browse/HADOOP-18987
> Project: Hadoop Common
> Issue Type: Improvement
> Components: documentation
> Affects Versions: 3.3.6
> Reporter: Dieter De Paepe
> Priority: Minor
> Labels: pull-request-available
>
> I noticed a lot of inconsistencies, typos and informal statements in the
> "formal" FileSystem API definition
> ([https://hadoop.apache.org/docs/stable/hadoop-project-dist/hadoop-common/filesystem/index.html)]
> Creating this ticket to link my PR against.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
