Skip to content

Commit

Permalink
ARROW-8145: [C++] Rename FileSystem::GetTargetInfos to GetFileInfo
Browse files Browse the repository at this point in the history
Closes #6688 from pitrou/ARROW-8145-rename-get-target-infos

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
  • Loading branch information
pitrou committed Mar 23, 2020
1 parent c43e235 commit a87b26c
Show file tree
Hide file tree
Showing 31 changed files with 240 additions and 235 deletions.
6 changes: 3 additions & 3 deletions c_glib/arrow-glib/file-system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ garrow_file_system_get_target_info(GArrowFileSystem *file_system,
GError **error)
{
auto arrow_file_system = garrow_file_system_get_raw(file_system);
auto arrow_result = arrow_file_system->GetTargetInfo(path);
auto arrow_result = arrow_file_system->GetFileInfo(path);
if (garrow::check(error, arrow_result, "[file-system][get-target-info]")) {
const auto &arrow_file_info = *arrow_result;
return garrow_file_info_new_raw(arrow_file_info);
Expand Down Expand Up @@ -712,7 +712,7 @@ garrow_file_system_get_target_infos_paths(GArrowFileSystem *file_system,
for (gsize i = 0; i < n_paths; ++i) {
arrow_paths.push_back(paths[i]);
}
return garrow_file_infos_new(arrow_file_system->GetTargetInfos(arrow_paths),
return garrow_file_infos_new(arrow_file_system->GetFileInfo(arrow_paths),
error,
"[file-system][get-target-infos][paths]");
}
Expand Down Expand Up @@ -742,7 +742,7 @@ garrow_file_system_get_target_infos_selector(GArrowFileSystem *file_system,
auto arrow_file_system = garrow_file_system_get_raw(file_system);
const auto &arrow_file_selector =
GARROW_FILE_SELECTOR_GET_PRIVATE(file_selector)->file_selector;
return garrow_file_infos_new(arrow_file_system->GetTargetInfos(arrow_file_selector),
return garrow_file_infos_new(arrow_file_system->GetFileInfo(arrow_file_selector),
error,
"[file-system][get-target-infos][selector]");
}
Expand Down
6 changes: 3 additions & 3 deletions cpp/src/arrow/dataset/discovery.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ Result<fs::PathForest> FileSystemDatasetFactory::Filter(
Result<std::shared_ptr<DatasetFactory>> FileSystemDatasetFactory::Make(
std::shared_ptr<fs::FileSystem> filesystem, const std::vector<std::string>& paths,
std::shared_ptr<FileFormat> format, FileSystemFactoryOptions options) {
ARROW_ASSIGN_OR_RAISE(auto files, filesystem->GetTargetInfos(paths));
ARROW_ASSIGN_OR_RAISE(auto files, filesystem->GetFileInfo(paths));
ARROW_ASSIGN_OR_RAISE(auto forest, fs::PathForest::Make(std::move(files)));

std::unordered_set<fs::FileInfo, fs::FileInfo::ByPath> missing;
Expand All @@ -167,7 +167,7 @@ Result<std::shared_ptr<DatasetFactory>> FileSystemDatasetFactory::Make(

for (auto&& path :
fs::internal::AncestorsFromBasePath(parent_path, ref.info().path())) {
ARROW_ASSIGN_OR_RAISE(auto file, filesystem->GetTargetInfo(std::move(path)));
ARROW_ASSIGN_OR_RAISE(auto file, filesystem->GetFileInfo(std::move(path)));
missing.insert(std::move(file));
}
return Status::OK();
Expand All @@ -187,7 +187,7 @@ Result<std::shared_ptr<DatasetFactory>> FileSystemDatasetFactory::Make(
Result<std::shared_ptr<DatasetFactory>> FileSystemDatasetFactory::Make(
std::shared_ptr<fs::FileSystem> filesystem, fs::FileSelector selector,
std::shared_ptr<FileFormat> format, FileSystemFactoryOptions options) {
ARROW_ASSIGN_OR_RAISE(auto files, filesystem->GetTargetInfos(selector));
ARROW_ASSIGN_OR_RAISE(auto files, filesystem->GetFileInfo(selector));

ARROW_ASSIGN_OR_RAISE(auto forest, fs::PathForest::Make(std::move(files)));

Expand Down
22 changes: 10 additions & 12 deletions cpp/src/arrow/filesystem/filesystem.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,12 @@ FileSystem::~FileSystem() {}

Result<std::string> FileSystem::NormalizePath(std::string path) { return path; }

Result<std::vector<FileInfo>> FileSystem::GetTargetInfos(
Result<std::vector<FileInfo>> FileSystem::GetFileInfo(
const std::vector<std::string>& paths) {
std::vector<FileInfo> res;
res.reserve(paths.size());
for (const auto& path : paths) {
ARROW_ASSIGN_OR_RAISE(FileInfo info, GetTargetInfo(path));
ARROW_ASSIGN_OR_RAISE(FileInfo info, GetFileInfo(path));
res.push_back(std::move(info));
}
return res;
Expand Down Expand Up @@ -190,17 +190,16 @@ Result<std::string> SubTreeFileSystem::NormalizePath(std::string path) {
return StripBase(std::move(normalized));
}

Result<FileInfo> SubTreeFileSystem::GetTargetInfo(const std::string& path) {
ARROW_ASSIGN_OR_RAISE(FileInfo info, base_fs_->GetTargetInfo(PrependBase(path)));
Result<FileInfo> SubTreeFileSystem::GetFileInfo(const std::string& path) {
ARROW_ASSIGN_OR_RAISE(FileInfo info, base_fs_->GetFileInfo(PrependBase(path)));
RETURN_NOT_OK(FixInfo(&info));
return info;
}

Result<std::vector<FileInfo>> SubTreeFileSystem::GetTargetInfos(
const FileSelector& select) {
Result<std::vector<FileInfo>> SubTreeFileSystem::GetFileInfo(const FileSelector& select) {
auto selector = select;
selector.base_dir = PrependBase(selector.base_dir);
ARROW_ASSIGN_OR_RAISE(auto infos, base_fs_->GetTargetInfos(selector));
ARROW_ASSIGN_OR_RAISE(auto infos, base_fs_->GetFileInfo(selector));
for (auto& info : infos) {
RETURN_NOT_OK(FixInfo(&info));
}
Expand Down Expand Up @@ -289,15 +288,14 @@ SlowFileSystem::SlowFileSystem(std::shared_ptr<FileSystem> base_fs,
double average_latency, int32_t seed)
: base_fs_(base_fs), latencies_(io::LatencyGenerator::Make(average_latency, seed)) {}

Result<FileInfo> SlowFileSystem::GetTargetInfo(const std::string& path) {
Result<FileInfo> SlowFileSystem::GetFileInfo(const std::string& path) {
latencies_->Sleep();
return base_fs_->GetTargetInfo(path);
return base_fs_->GetFileInfo(path);
}

Result<std::vector<FileInfo>> SlowFileSystem::GetTargetInfos(
const FileSelector& selector) {
Result<std::vector<FileInfo>> SlowFileSystem::GetFileInfo(const FileSelector& selector) {
latencies_->Sleep();
return base_fs_->GetTargetInfos(selector);
return base_fs_->GetFileInfo(selector);
}

Status SlowFileSystem::CreateDir(const std::string& path, bool recursive) {
Expand Down
22 changes: 10 additions & 12 deletions cpp/src/arrow/filesystem/filesystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,16 @@ class ARROW_EXPORT FileSystem {
/// A nonexistent or unreachable file returns an Ok status and
/// has a FileType of value NotFound. An error status indicates
/// a truly exceptional condition (low-level I/O error, etc.).
virtual Result<FileInfo> GetTargetInfo(const std::string& path) = 0;
virtual Result<FileInfo> GetFileInfo(const std::string& path) = 0;
/// Same, for many targets at once.
virtual Result<std::vector<FileInfo>> GetTargetInfos(
virtual Result<std::vector<FileInfo>> GetFileInfo(
const std::vector<std::string>& paths);
/// Same, according to a selector.
///
/// The selector's base directory will not be part of the results, even if
/// it exists.
/// If it doesn't exist, see `FileSelector::allow_non_existent`.
virtual Result<std::vector<FileInfo>> GetTargetInfos(const FileSelector& select) = 0;
/// If it doesn't exist, see `FileSelector::allow_not_found`.
virtual Result<std::vector<FileInfo>> GetFileInfo(const FileSelector& select) = 0;

/// Create a directory and subdirectories.
///
Expand Down Expand Up @@ -262,11 +262,10 @@ class ARROW_EXPORT SubTreeFileSystem : public FileSystem {
Result<std::string> NormalizePath(std::string path) override;

/// \cond FALSE
using FileSystem::GetTargetInfo;
using FileSystem::GetTargetInfos;
using FileSystem::GetFileInfo;
/// \endcond
Result<FileInfo> GetTargetInfo(const std::string& path) override;
Result<std::vector<FileInfo>> GetTargetInfos(const FileSelector& select) override;
Result<FileInfo> GetFileInfo(const std::string& path) override;
Result<std::vector<FileInfo>> GetFileInfo(const FileSelector& select) override;

Status CreateDir(const std::string& path, bool recursive = true) override;

Expand Down Expand Up @@ -315,10 +314,9 @@ class ARROW_EXPORT SlowFileSystem : public FileSystem {

std::string type_name() const override { return "slow"; }

using FileSystem::GetTargetInfo;
using FileSystem::GetTargetInfos;
Result<FileInfo> GetTargetInfo(const std::string& path) override;
Result<std::vector<FileInfo>> GetTargetInfos(const FileSelector& select) override;
using FileSystem::GetFileInfo;
Result<FileInfo> GetFileInfo(const std::string& path) override;
Result<std::vector<FileInfo>> GetFileInfo(const FileSelector& select) override;

Status CreateDir(const std::string& path, bool recursive = true) override;

Expand Down
43 changes: 21 additions & 22 deletions cpp/src/arrow/filesystem/filesystem_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -362,29 +362,29 @@ TEST_F(TestMockFS, DeleteFile) {
CheckFiles({});
}

TEST_F(TestMockFS, GetTargetInfo) {
TEST_F(TestMockFS, GetFileInfo) {
ASSERT_OK(fs_->CreateDir("AB/CD"));
CreateFile("AB/CD/ef", "some data");

FileInfo info;
ASSERT_OK_AND_ASSIGN(info, fs_->GetTargetInfo("AB"));
ASSERT_OK_AND_ASSIGN(info, fs_->GetFileInfo("AB"));
AssertFileInfo(info, "AB", FileType::Directory, time_);
ASSERT_EQ(info.base_name(), "AB");
ASSERT_OK_AND_ASSIGN(info, fs_->GetTargetInfo("AB/CD/ef"));
ASSERT_OK_AND_ASSIGN(info, fs_->GetFileInfo("AB/CD/ef"));
AssertFileInfo(info, "AB/CD/ef", FileType::File, time_, 9);
ASSERT_EQ(info.base_name(), "ef");

// Invalid path
ASSERT_RAISES(Invalid, fs_->GetTargetInfo("//foo//bar//baz//"));
ASSERT_RAISES(Invalid, fs_->GetFileInfo("//foo//bar//baz//"));
}

TEST_F(TestMockFS, GetTargetInfosVector) {
TEST_F(TestMockFS, GetFileInfoVector) {
ASSERT_OK(fs_->CreateDir("AB/CD"));
CreateFile("AB/CD/ef", "some data");

std::vector<FileInfo> infos;
ASSERT_OK_AND_ASSIGN(
infos, fs_->GetTargetInfos({"AB", "AB/CD", "AB/zz", "zz", "XX/zz", "AB/CD/ef"}));
infos, fs_->GetFileInfo({"AB", "AB/CD", "AB/zz", "zz", "XX/zz", "AB/CD/ef"}));
ASSERT_EQ(infos.size(), 6);
AssertFileInfo(infos[0], "AB", FileType::Directory, time_);
AssertFileInfo(infos[1], "AB/CD", FileType::Directory, time_);
Expand All @@ -394,31 +394,31 @@ TEST_F(TestMockFS, GetTargetInfosVector) {
AssertFileInfo(infos[5], "AB/CD/ef", FileType::File, time_, 9);

// Invalid path
ASSERT_RAISES(Invalid, fs_->GetTargetInfos({"AB", "AB/CD", "//foo//bar//baz//"}));
ASSERT_RAISES(Invalid, fs_->GetFileInfo({"AB", "AB/CD", "//foo//bar//baz//"}));
}

TEST_F(TestMockFS, GetTargetInfosSelector) {
TEST_F(TestMockFS, GetFileInfoSelector) {
ASSERT_OK(fs_->CreateDir("AB/CD"));
CreateFile("ab", "data");

FileSelector s;
s.base_dir = "";
std::vector<FileInfo> infos;
ASSERT_OK_AND_ASSIGN(infos, fs_->GetTargetInfos(s));
ASSERT_OK_AND_ASSIGN(infos, fs_->GetFileInfo(s));
ASSERT_EQ(infos.size(), 2);
AssertFileInfo(infos[0], "AB", FileType::Directory, time_);
AssertFileInfo(infos[1], "ab", FileType::File, time_, 4);

s.recursive = true;
ASSERT_OK_AND_ASSIGN(infos, fs_->GetTargetInfos(s));
ASSERT_OK_AND_ASSIGN(infos, fs_->GetFileInfo(s));
ASSERT_EQ(infos.size(), 3);
AssertFileInfo(infos[0], "AB", FileType::Directory, time_);
AssertFileInfo(infos[1], "AB/CD", FileType::Directory, time_);
AssertFileInfo(infos[2], "ab", FileType::File, time_, 4);

// Invalid path
s.base_dir = "//foo//bar//baz//";
ASSERT_RAISES(Invalid, fs_->GetTargetInfos(s));
ASSERT_RAISES(Invalid, fs_->GetFileInfo(s));
}

TEST_F(TestMockFS, OpenOutputStream) {
Expand Down Expand Up @@ -655,7 +655,7 @@ TEST_F(TestSubTreeFileSystem, OpenAppendStream) {
CheckFiles({{"sub/tree/ab", time_, "some data"}});
}

TEST_F(TestSubTreeFileSystem, GetTargetInfo) {
TEST_F(TestSubTreeFileSystem, GetFileInfo) {
ASSERT_OK(subfs_->CreateDir("AB/CD"));

AssertFileInfo(subfs_.get(), "AB", FileType::Directory, time_);
Expand All @@ -667,23 +667,22 @@ TEST_F(TestSubTreeFileSystem, GetTargetInfo) {
AssertFileInfo(subfs_.get(), "nonexistent", FileType::NotFound);
}

TEST_F(TestSubTreeFileSystem, GetTargetInfosVector) {
TEST_F(TestSubTreeFileSystem, GetFileInfoVector) {
std::vector<FileInfo> infos;

ASSERT_OK(subfs_->CreateDir("AB/CD"));
CreateFile("ab", "data");
CreateFile("AB/cd", "other data");

ASSERT_OK_AND_ASSIGN(infos,
subfs_->GetTargetInfos({"ab", "AB", "AB/cd", "nonexistent"}));
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetFileInfo({"ab", "AB", "AB/cd", "nonexistent"}));
ASSERT_EQ(infos.size(), 4);
AssertFileInfo(infos[0], "ab", FileType::File, time_, 4);
AssertFileInfo(infos[1], "AB", FileType::Directory, time_);
AssertFileInfo(infos[2], "AB/cd", FileType::File, time_, 10);
AssertFileInfo(infos[3], "nonexistent", FileType::NotFound);
}

TEST_F(TestSubTreeFileSystem, GetTargetInfosSelector) {
TEST_F(TestSubTreeFileSystem, GetFileInfoSelector) {
std::vector<FileInfo> infos;
FileSelector selector;

Expand All @@ -694,27 +693,27 @@ TEST_F(TestSubTreeFileSystem, GetTargetInfosSelector) {

selector.base_dir = "AB";
selector.recursive = false;
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetTargetInfos(selector));
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetFileInfo(selector));
ASSERT_EQ(infos.size(), 2);
AssertFileInfo(infos[0], "AB/CD", FileType::Directory, time_);
AssertFileInfo(infos[1], "AB/cd", FileType::File, time_, 5);

selector.recursive = true;
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetTargetInfos(selector));
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetFileInfo(selector));
ASSERT_EQ(infos.size(), 3);
AssertFileInfo(infos[0], "AB/CD", FileType::Directory, time_);
AssertFileInfo(infos[1], "AB/CD/ef", FileType::File, time_, 6);
AssertFileInfo(infos[2], "AB/cd", FileType::File, time_, 5);

selector.base_dir = "";
selector.recursive = false;
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetTargetInfos(selector));
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetFileInfo(selector));
ASSERT_EQ(infos.size(), 2);
AssertFileInfo(infos[0], "AB", FileType::Directory, time_);
AssertFileInfo(infos[1], "ab", FileType::File, time_, 4);

selector.recursive = true;
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetTargetInfos(selector));
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetFileInfo(selector));
ASSERT_EQ(infos.size(), 5);
AssertFileInfo(infos[0], "AB", FileType::Directory, time_);
AssertFileInfo(infos[1], "AB/CD", FileType::Directory, time_);
Expand All @@ -723,9 +722,9 @@ TEST_F(TestSubTreeFileSystem, GetTargetInfosSelector) {
AssertFileInfo(infos[4], "ab", FileType::File, time_, 4);

selector.base_dir = "nonexistent";
ASSERT_RAISES(IOError, subfs_->GetTargetInfos(selector));
ASSERT_RAISES(IOError, subfs_->GetFileInfo(selector));
selector.allow_not_found = true;
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetTargetInfos(selector));
ASSERT_OK_AND_ASSIGN(infos, subfs_->GetFileInfo(selector));
ASSERT_EQ(infos.size(), 0);
}

Expand Down
19 changes: 9 additions & 10 deletions cpp/src/arrow/filesystem/hdfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class HadoopFileSystem::Impl {
return Status::OK();
}

Result<FileInfo> GetTargetInfo(const std::string& path) {
Result<FileInfo> GetFileInfo(const std::string& path) {
FileInfo info;
io::HdfsPathInfo path_info;
auto status = client_->GetPathInfo(path, &path_info);
Expand All @@ -84,7 +84,7 @@ class HadoopFileSystem::Impl {
Status st = client_->ListDirectory(path, &children);
if (!st.ok()) {
if (select.allow_not_found) {
ARROW_ASSIGN_OR_RAISE(auto info, GetTargetInfo(path));
ARROW_ASSIGN_OR_RAISE(auto info, GetFileInfo(path));
if (info.type() == FileType::NotFound) {
return Status::OK();
}
Expand Down Expand Up @@ -112,7 +112,7 @@ class HadoopFileSystem::Impl {
return Status::OK();
}

Result<std::vector<FileInfo>> GetTargetInfos(const FileSelector& select) {
Result<std::vector<FileInfo>> GetFileInfo(const FileSelector& select) {
std::vector<FileInfo> results;

std::string wd;
Expand All @@ -125,10 +125,10 @@ class HadoopFileSystem::Impl {
wd = wd_uri.path();
}

ARROW_ASSIGN_OR_RAISE(auto info, GetTargetInfo(select.base_dir));
ARROW_ASSIGN_OR_RAISE(auto info, GetFileInfo(select.base_dir));
if (info.type() == FileType::File) {
return Status::Invalid(
"GetTargetInfos expects base_dir of selector to be a directory, while '",
"GetFileInfo expects base_dir of selector to be a directory, while '",
select.base_dir, "' is a file");
}
RETURN_NOT_OK(StatSelector(wd, select.base_dir, select, 0, &results));
Expand Down Expand Up @@ -326,13 +326,12 @@ Result<std::shared_ptr<HadoopFileSystem>> HadoopFileSystem::Make(
return ptr;
}

Result<FileInfo> HadoopFileSystem::GetTargetInfo(const std::string& path) {
return impl_->GetTargetInfo(path);
Result<FileInfo> HadoopFileSystem::GetFileInfo(const std::string& path) {
return impl_->GetFileInfo(path);
}

Result<std::vector<FileInfo>> HadoopFileSystem::GetTargetInfos(
const FileSelector& select) {
return impl_->GetTargetInfos(select);
Result<std::vector<FileInfo>> HadoopFileSystem::GetFileInfo(const FileSelector& select) {
return impl_->GetFileInfo(select);
}

Status HadoopFileSystem::CreateDir(const std::string& path, bool recursive) {
Expand Down
7 changes: 3 additions & 4 deletions cpp/src/arrow/filesystem/hdfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,10 @@ class ARROW_EXPORT HadoopFileSystem : public FileSystem {
std::string type_name() const override { return "hdfs"; }

/// \cond FALSE
using FileSystem::GetTargetInfo;
using FileSystem::GetTargetInfos;
using FileSystem::GetFileInfo;
/// \endcond
Result<FileInfo> GetTargetInfo(const std::string& path) override;
Result<std::vector<FileInfo>> GetTargetInfos(const FileSelector& select) override;
Result<FileInfo> GetFileInfo(const std::string& path) override;
Result<std::vector<FileInfo>> GetFileInfo(const FileSelector& select) override;

Status CreateDir(const std::string& path, bool recursive = true) override;

Expand Down
Loading

0 comments on commit a87b26c

Please sign in to comment.