Copilot commented on code in PR #45998:
URL: https://github.com/apache/arrow/pull/45998#discussion_r3374394620
##########
cpp/src/arrow/filesystem/hdfs.h:
##########
@@ -19,21 +19,36 @@
#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 {
+namespace internal {
+class HdfsReadableFile;
+class HdfsOutputStream;
+
+struct HdfsPathInfo;
+} // namespace internal
+
+struct HdfsConnectionConfig {
+ std::string host;
+ int port;
+ std::string user;
+ std::string kerb_ticket;
+ std::unordered_map<std::string, std::string> extra_conf;
+};
Review Comment:
`HdfsConnectionConfig::port` is currently left uninitialized by default
construction, so `HdfsOptions options;` followed by
`HadoopFileSystem::Make(options, ...)` can pass an indeterminate port into
`BuilderSetNameNodePort`, which is undefined behavior. Initialize the port
member to a known default (0 is already treated as "default port" in
`HdfsOptions::FromUri`).
##########
cpp/src/arrow/filesystem/hdfs.h:
##########
@@ -88,10 +100,49 @@ class ARROW_EXPORT HadoopFileSystem : public FileSystem {
Status DeleteFile(const std::string& path) override;
+ Status MakeDirectory(const std::string& path);
+
+ bool Exists(const std::string& path);
+
+ Status GetPathInfoStatus(const std::string& path, internal::HdfsPathInfo*
info);
+
+ Status ListDirectory(const std::string& path,
+ std::vector<internal::HdfsPathInfo>* listing);
Review Comment:
Several new public `HadoopFileSystem` methods expose `arrow::fs::internal`
types in their signatures (e.g. `internal::HdfsPathInfo`,
`internal::HdfsReadableFile`, `internal::HdfsOutputStream`). Since this header
is part of the public filesystem API, this effectively makes those `internal`
types part of the public ABI/API surface and constrains future refactors.
Consider either (a) making these types public (not in an `internal` namespace)
if they are intended for external consumption, or (b) keeping these helpers
private/hidden and exposing equivalent functionality via existing `FileSystem`
APIs and `FileInfo`/streams.
--
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]
