pitrou commented on code in PR #45998:
URL: https://github.com/apache/arrow/pull/45998#discussion_r3474213065
##########
cpp/src/arrow/filesystem/hdfs_internal_test.cc:
##########
Review Comment:
Is this file useful at all? `hdfs_test.cc` is supposed to exercise all
public filesystem APIs already.
##########
cpp/src/arrow/filesystem/hdfs_internal.cc:
##########
@@ -51,12 +52,20 @@
namespace arrow {
using internal::GetEnvVarNative;
+using internal::IOErrorFromErrno;
using internal::PlatformFilename;
-namespace io::internal {
+namespace fs::internal {
namespace {
+#define CHECK_FAILURE(RETURN_VALUE, WHAT) \
Review Comment:
Can you `#undef` this at the end of the file?
##########
cpp/src/arrow/filesystem/hdfs.h:
##########
@@ -19,21 +19,34 @@
#include <memory>
#include <string>
+#include <unordered_map>
#include <vector>
#include "arrow/filesystem/filesystem.h"
-#include "arrow/io/hdfs.h"
#include "arrow/util/uri.h"
namespace arrow::fs {
+class HdfsReadableFile;
+class HdfsOutputStream;
+
+struct HdfsPathInfo;
Review Comment:
I don't think we should expose these?
##########
cpp/src/arrow/filesystem/hdfs.h:
##########
@@ -88,10 +98,48 @@ class ARROW_EXPORT HadoopFileSystem : public FileSystem {
Status DeleteFile(const std::string& path) override;
+ Status MakeDirectory(const std::string& path);
Review Comment:
I understand these methods existed on the legacy HDFS filesystem class, but
I think we should only implement the standard `FileSystem` methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
