Skip to content

Commit ee0ac02

Browse files
teo-tsirpanisteo-tsirpanis
committed
Fix test failures.
1 parent 0fa81cc commit ee0ac02

File tree

7 files changed

+55
-41
lines changed

7 files changed

+55
-41
lines changed

‎test/support/src/vfs_helpers.cc‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,4 +536,25 @@ bool VFSTestSetup::is_legacy_rest() {
536536
return is_rest() && ctx_c->rest_client().rest_legacy();
537537
}
538538

539+
TileDBFSTest::TileDBFSTest(const std::vector<size_t>& test_tree)
540+
: VFSTestBase(test_tree, "tiledb://workspace/teamspace/", "rest-s3", true) {
541+
for (size_t i = 1; i <= test_tree_.size(); i++) {
542+
sm::URI path = temp_dir_.join_path("subdir_" + std::to_string(i));
543+
// VFS::create_dir is a no-op for Azure; Just create objects.
544+
if (test_tree_[i - 1] > 0) {
545+
// Do not include an empty prefix in expected results.
546+
expected_results_.emplace_back(path.to_string(), 0);
547+
}
548+
for (size_t j = 1; j <= test_tree_[i - 1]; j++) {
549+
auto object_uri = path.join_path("test_file_" + std::to_string(j));
550+
vfs_.touch(object_uri);
551+
std::string data(j * 10, 'a');
552+
vfs_.write(object_uri, data.data(), data.size());
553+
vfs_.close_file(object_uri).ok();
554+
expected_results_.emplace_back(object_uri.to_string(), data.size());
555+
}
556+
}
557+
std::sort(expected_results_.begin(), expected_results_.end());
558+
}
559+
539560
} // namespace tiledb::test

‎test/support/src/vfs_helpers.h‎

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,30 +1117,7 @@ class GCSTest : public VFSTestBase {
11171117
/** Stub test object for TileDB FS functionality. */
11181118
class TileDBFSTest : public VFSTestBase {
11191119
public:
1120-
explicit TileDBFSTest(const std::vector<size_t>& test_tree)
1121-
: VFSTestBase(
1122-
test_tree,
1123-
"tiledb://unit-workspace/unit-teamspace/",
1124-
"rest-s3",
1125-
true) {
1126-
for (size_t i = 1; i <= test_tree_.size(); i++) {
1127-
sm::URI path = temp_dir_.join_path("subdir_" + std::to_string(i));
1128-
// VFS::create_dir is a no-op for Azure; Just create objects.
1129-
if (test_tree_[i - 1] > 0) {
1130-
// Do not include an empty prefix in expected results.
1131-
expected_results_.emplace_back(path.to_string(), 0);
1132-
}
1133-
for (size_t j = 1; j <= test_tree_[i - 1]; j++) {
1134-
auto object_uri = path.join_path("test_file_" + std::to_string(j));
1135-
vfs_.touch(object_uri);
1136-
std::string data(j * 10, 'a');
1137-
vfs_.write(object_uri, data.data(), data.size());
1138-
vfs_.close_file(object_uri).ok();
1139-
expected_results_.emplace_back(object_uri.to_string(), data.size());
1140-
}
1141-
}
1142-
std::sort(expected_results_.begin(), expected_results_.end());
1143-
}
1120+
explicit TileDBFSTest(const std::vector<size_t>& test_tree);
11441121
};
11451122

11461123
/** Stub test object for tiledb::sm::GS functionality. */

‎tiledb/sm/filesystem/gcs.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -487,8 +487,7 @@ std::vector<directory_entry> GCS::ls_with_sizes(
487487
}
488488

489489
auto& results = object_metadata.value();
490-
const std::string gcs_prefix =
491-
uri_dir.backend_name() == "gcs" ? "gcs://" : "gs://";
490+
const std::string gcs_prefix = uri_dir.backend_name() + "://";
492491

493492
if (absl::holds_alternative<google::cloud::storage::ObjectMetadata>(
494493
results)) {

‎tiledb/sm/filesystem/s3.cc‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,7 @@ std::vector<directory_entry> S3::ls_with_sizes(
996996

997997
const auto prefix_dir = prefix.add_trailing_slash();
998998

999+
auto protocol = prefix.backend_name() + "://";
9991000
auto prefix_str = prefix_dir.to_string();
10001001
Aws::Http::URI aws_uri = prefix_str.c_str();
10011002
std::string aws_auth(aws_uri.GetAuthority());
@@ -1029,7 +1030,7 @@ std::vector<directory_entry> S3::ls_with_sizes(
10291030
std::string file(object.GetKey());
10301031
uint64_t size = object.GetSize();
10311032
entries.emplace_back(
1032-
"s3://" + aws_auth + add_front_slash(file), size, false);
1033+
protocol + aws_auth + add_front_slash(file), size, false);
10331034
}
10341035

10351036
for (const auto& object :
@@ -1038,7 +1039,7 @@ std::vector<directory_entry> S3::ls_with_sizes(
10381039
// For "directories" it doesn't seem possible to get a shallow size in
10391040
// S3, so the size of such an entry will be 0 in S3.
10401041
entries.emplace_back(
1041-
"s3://" + aws_auth + add_front_slash(remove_trailing_slash(file)),
1042+
protocol + aws_auth + add_front_slash(remove_trailing_slash(file)),
10421043
0,
10431044
true);
10441045
}
@@ -1094,7 +1095,7 @@ uint64_t S3::file_size(const URI& uri) const {
10941095
}
10951096
throw S3Exception(
10961097
std::string(
1097-
"Cannot retrieve S3 object size; Error while listing file s3://") +
1098+
"Cannot retrieve S3 object size; Error while listing file ") +
10981099
uri.to_string() + outcome_error_message(head_object_outcome) +
10991100
additional_context);
11001101
}
@@ -2044,7 +2045,8 @@ S3Scanner::S3Scanner(
20442045
bool recursive,
20452046
int max_keys)
20462047
: LsScanner(prefix, std::move(result_filter), recursive)
2047-
, client_(client) {
2048+
, client_(client)
2049+
, protocol_(prefix.backend_name() + "://") {
20482050
const auto prefix_dir = prefix.add_trailing_slash();
20492051
auto prefix_str = prefix_dir.to_string();
20502052
Aws::Http::URI aws_uri = prefix_str.c_str();
@@ -2071,7 +2073,8 @@ S3Scanner::S3Scanner(
20712073
bool recursive,
20722074
int max_keys)
20732075
: LsScanner(prefix, std::move(result_filter), recursive)
2074-
, client_(client) {
2076+
, client_(client)
2077+
, protocol_(prefix.backend_name() + "://") {
20752078
const auto prefix_dir = prefix.add_trailing_slash();
20762079
auto prefix_str = prefix_dir.to_string();
20772080
Aws::Http::URI aws_uri = prefix_str.c_str();
@@ -2173,11 +2176,12 @@ void S3Scanner::next(typename Iterator::pointer& ptr) {
21732176
ptr = fetch_results();
21742177
}
21752178

2176-
std::string bucket = "s3://" + std::string(list_objects_request_.GetBucket());
2179+
std::string bucket =
2180+
protocol_ + std::string(list_objects_request_.GetBucket());
21772181
while (ptr != end_) {
21782182
auto object = *ptr;
21792183
uint64_t size = object.GetSize();
2180-
// The object key does not contain s3:// prefix or the bucket name.
2184+
// The object key does not contain protocol prefix or the bucket name.
21812185
std::string path =
21822186
bucket + S3::add_front_slash(std::string(object.GetKey()));
21832187

‎tiledb/sm/filesystem/s3.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,9 @@ class S3Scanner : public LsScanner {
521521
/** Iterators for the current objects fetched from S3. */
522522
typename Iterator::pointer begin_, end_;
523523

524+
/** The input URI's prococol (might be s3:// or tiledb://). */
525+
const std::string protocol_;
526+
524527
/** The current request being scanned. */
525528
Aws::S3::Model::ListObjectsV2Request list_objects_request_;
526529
/** The current request outcome being scanned. */

‎tiledb/sm/filesystem/uri.cc‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -409,11 +409,7 @@ std::string URI::to_path(const std::string& uri) {
409409
}
410410

411411
std::string URI::backend_name() const {
412-
if (is_tiledb(uri_)) {
413-
return "";
414-
} else {
415-
return uri_.substr(0, uri_.find_first_of(':'));
416-
}
412+
return uri_.substr(0, uri_.find_first_of(':'));
417413
}
418414

419415
std::string URI::to_path() const {

‎tiledb/sm/filesystem/vfs.cc‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ Config VFS::config() const {
218218
}
219219

220220
void VFS::create_dir(const URI& uri) const {
221-
if (!uri.is_s3() && !uri.is_azure() && !uri.is_gcs()) {
221+
if (!(uri.is_s3() || uri.is_azure() && uri.is_gcs() || uri.is_tiledb())) {
222222
if (this->is_dir(uri))
223223
return;
224224
}
@@ -326,7 +326,7 @@ void VFS::remove_files(
326326
}
327327

328328
uint64_t VFS::max_parallel_ops(const URI& uri) const {
329-
if (uri.is_s3()) {
329+
if (uri.is_s3() || uri.is_tiledb()) {
330330
return config_.get<uint64_t>("vfs.s3.max_parallel_ops", Config::must_find);
331331
} else if (uri.is_azure()) {
332332
return config_.get<uint64_t>(
@@ -375,7 +375,8 @@ std::vector<directory_entry> VFS::ls_with_sizes(const URI& parent) const {
375375
// Noop if `parent` is not a directory, do not error out.
376376
// For S3, GCS and Azure, `ls` on a non-directory will just
377377
// return an empty `uris` vector.
378-
if (!(parent.is_s3() || parent.is_gcs() || parent.is_azure())) {
378+
if (!(parent.is_s3() || parent.is_gcs() || parent.is_azure() ||
379+
parent.is_tiledb())) {
379380
if (!this->is_dir(parent)) {
380381
return {};
381382
}
@@ -608,6 +609,8 @@ bool VFS::supports_uri_scheme(const URI& uri) const {
608609
return supports_fs(Filesystem::AZURE);
609610
} else if (uri.is_gcs()) {
610611
return supports_fs(Filesystem::GCS);
612+
} else if (uri.is_tiledb()) {
613+
return supports_fs(Filesystem::TILEDBFS);
611614
} else {
612615
return true;
613616
}
@@ -658,6 +661,17 @@ Status VFS::open_file(const URI& uri, VFSMode mode) {
658661
throw BuiltWithout("GCS");
659662
}
660663
}
664+
if (uri.is_tiledb()) {
665+
if constexpr (s3_enabled) {
666+
throw VFSException(
667+
"Cannot open file '" + uri.to_string() +
668+
"'; TileDB FS does not support append mode");
669+
} else {
670+
throw VFSException(
671+
"TileDB was built without S3 support, which is required for "
672+
"TileDB FS");
673+
}
674+
}
661675
break;
662676
}
663677

0 commit comments

Comments
 (0)