Skip to content

Commit 115d273

Browse files
jbmscopybara-github
authored andcommitted
Add DriverKindRegistry and improve unregistered kvstore error message
Previously, if the user attempted to create a KvStore spec using a TensorStore driver id, a generic "unregistered id" error was returned. With this change, the error message will indicate that a TensorStore id rather than a KvStore id was specified. In a subsequent commit, the reverse case (creating a TensorStore spec from a KvStore driver id) will handled (not as an error) via the new format auto-detection driver. PiperOrigin-RevId: 755690932 Change-Id: Ia712f089a45aee2931ef01f19046f94426c02544
1 parent c624bbd commit 115d273

9 files changed

Lines changed: 198 additions & 11 deletions

File tree

tensorstore/driver/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ tensorstore_cc_library(
138138
"//tensorstore/index_space:transform_broadcastable_array",
139139
"//tensorstore/index_space:transformed_array",
140140
"//tensorstore/internal:context_binding",
141+
"//tensorstore/internal:driver_kind_registry",
141142
"//tensorstore/internal:intrusive_ptr",
142143
"//tensorstore/internal:json_registry",
143144
"//tensorstore/internal:lock_collection",

tensorstore/driver/registry.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "tensorstore/context.h"
3030
#include "tensorstore/driver/driver_spec.h"
3131
#include "tensorstore/internal/context_binding.h"
32+
#include "tensorstore/internal/driver_kind_registry.h"
3233
#include "tensorstore/internal/json_binding/bindable.h"
3334
#include "tensorstore/internal/json_registry.h"
3435
#include "tensorstore/json_serialization_options.h"
@@ -153,6 +154,8 @@ class DriverRegistration {
153154
: DriverRegistration(
154155
/*aliases=*/tensorstore::span<const std::string_view>{}) {}
155156
DriverRegistration(tensorstore::span<const std::string_view> aliases) {
157+
internal::RegisterDriverKind(Spec::id, internal::DriverKind::kTensorStore,
158+
aliases);
156159
GetDriverRegistry().Register<Spec>(
157160
Spec::id, internal_json_binding::DefaultBinder<>, aliases);
158161
serialization::Register<DriverSpecPtr, Spec>();

tensorstore/internal/BUILD

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,20 @@ tensorstore_cc_library(
277277
],
278278
)
279279

280+
tensorstore_cc_library(
281+
name = "driver_kind_registry",
282+
srcs = ["driver_kind_registry.cc"],
283+
hdrs = ["driver_kind_registry.h"],
284+
deps = [
285+
"//tensorstore/util:span",
286+
"@abseil-cpp//absl/base:core_headers",
287+
"@abseil-cpp//absl/base:no_destructor",
288+
"@abseil-cpp//absl/container:flat_hash_map",
289+
"@abseil-cpp//absl/log:absl_log",
290+
"@abseil-cpp//absl/synchronization",
291+
],
292+
)
293+
280294
tensorstore_cc_library(
281295
name = "element_copy_function",
282296
hdrs = ["element_copy_function.h"],
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright 2025 The TensorStore Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#include "tensorstore/internal/driver_kind_registry.h"
16+
17+
#include <optional>
18+
#include <ostream>
19+
#include <string>
20+
#include <string_view>
21+
22+
#include "absl/base/no_destructor.h"
23+
#include "absl/base/optimization.h"
24+
#include "absl/base/thread_annotations.h"
25+
#include "absl/container/flat_hash_map.h"
26+
#include "absl/log/absl_log.h"
27+
#include "absl/synchronization/mutex.h"
28+
#include "tensorstore/util/span.h"
29+
30+
namespace tensorstore {
31+
namespace internal {
32+
33+
namespace {
34+
struct DriverKindRegistry {
35+
absl::Mutex mutex;
36+
absl::flat_hash_map<std::string, DriverKind> driver_kinds
37+
ABSL_GUARDED_BY(mutex);
38+
};
39+
40+
DriverKindRegistry& GetDriverKindRegistry() {
41+
static absl::NoDestructor<DriverKindRegistry> registry;
42+
return *registry;
43+
}
44+
} // namespace
45+
46+
void RegisterDriverKind(std::string_view driver_id, DriverKind driver_kind) {
47+
auto& registry = GetDriverKindRegistry();
48+
absl::MutexLock lock(&registry.mutex);
49+
if (auto result = registry.driver_kinds.emplace(driver_id, driver_kind);
50+
!result.second) {
51+
ABSL_LOG(FATAL) << driver_id << " already registered as "
52+
<< result.first->second;
53+
}
54+
}
55+
56+
void RegisterDriverKind(std::string_view id, DriverKind kind,
57+
tensorstore::span<const std::string_view> aliases) {
58+
RegisterDriverKind(id, kind);
59+
for (auto alias : aliases) {
60+
RegisterDriverKind(alias, kind);
61+
}
62+
}
63+
64+
std::optional<DriverKind> GetDriverKind(std::string_view id) {
65+
auto& registry = GetDriverKindRegistry();
66+
absl::MutexLock lock(&registry.mutex);
67+
auto it = registry.driver_kinds.find(id);
68+
if (it == registry.driver_kinds.end()) return std::nullopt;
69+
return it->second;
70+
}
71+
72+
std::string_view DriverKindToStringView(DriverKind x) {
73+
switch (x) {
74+
case DriverKind::kKvStore:
75+
return "kvstore";
76+
case DriverKind::kTensorStore:
77+
return "TensorStore";
78+
default:
79+
ABSL_UNREACHABLE();
80+
}
81+
}
82+
83+
std::ostream& operator<<(std::ostream& os, DriverKind x) {
84+
return os << DriverKindToStringView(x);
85+
}
86+
87+
} // namespace internal
88+
} // namespace tensorstore
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright 2025 The TensorStore Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#ifndef TENSORSTORE_INTERNAL_DRIVER_KIND_REGISTRY_H_
16+
#define TENSORSTORE_INTERNAL_DRIVER_KIND_REGISTRY_H_
17+
18+
#include <iosfwd>
19+
#include <optional>
20+
#include <string_view>
21+
22+
#include "tensorstore/util/span.h"
23+
24+
namespace tensorstore {
25+
namespace internal {
26+
27+
// Indicates the kind of a registered KvStore or TensorStore driver.
28+
//
29+
// The registry of driver kinds enables better error messages when a TensorStore
30+
// driver is accidentally specified when creating a KvStore spec, and enables
31+
// format auto-detection to be used implicitly when a KvStore driver is
32+
// specified when creating a TensorStore spec.
33+
enum class DriverKind {
34+
kKvStore,
35+
kTensorStore,
36+
};
37+
38+
// Returns the name of a driver kind for use in error messages.
39+
std::string_view DriverKindToStringView(DriverKind x);
40+
41+
template <typename Sink>
42+
void AbslStringify(Sink& sink, DriverKind x) {
43+
return sink.Append(DriverKindToStringView(x));
44+
}
45+
std::ostream& operator<<(std::ostream& os, DriverKind x);
46+
47+
// Registers a driver identifier. These are called automatically by the kvstore
48+
// and TensorStore driver registration classes.
49+
void RegisterDriverKind(std::string_view id, DriverKind kind);
50+
void RegisterDriverKind(std::string_view id, DriverKind kind,
51+
tensorstore::span<const std::string_view> aliases);
52+
53+
// Returns the kind of a registered driver id, or `std::nullopt` if not
54+
// registered.
55+
std::optional<DriverKind> GetDriverKind(std::string_view id);
56+
57+
} // namespace internal
58+
} // namespace tensorstore
59+
60+
#endif // TENSORSTORE_INTERNAL_DRIVER_KIND_REGISTRY_H_

tensorstore/kvstore/BUILD

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ tensorstore_cc_library(
176176
"//tensorstore:transaction",
177177
"//tensorstore/internal:compare",
178178
"//tensorstore/internal:context_binding",
179+
"//tensorstore/internal:driver_kind_registry",
179180
"//tensorstore/internal:intrusive_ptr",
180181
"//tensorstore/internal:json_registry",
181182
"//tensorstore/internal:mutex",
@@ -231,11 +232,8 @@ tensorstore_cc_test(
231232
deps = [
232233
":kvstore",
233234
"//tensorstore:context",
234-
"//tensorstore/util:future",
235-
"//tensorstore/util:result",
236-
"//tensorstore/util:status",
235+
"//tensorstore/driver/json",
237236
"//tensorstore/util:status_testutil",
238-
"//tensorstore/util:str_cat",
239237
"@abseil-cpp//absl/status",
240238
"@googletest//:gtest_main",
241239
],

tensorstore/kvstore/kvstore_test.cc

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,28 +12,34 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
#include "tensorstore/kvstore/kvstore.h"
16+
1517
#include <gmock/gmock.h>
1618
#include <gtest/gtest.h>
1719
#include "absl/status/status.h"
1820
#include "tensorstore/context.h"
19-
#include "tensorstore/kvstore/kvstore.h"
20-
#include "tensorstore/util/future.h"
21-
#include "tensorstore/util/result.h"
22-
#include "tensorstore/util/status.h"
2321
#include "tensorstore/util/status_testutil.h"
24-
#include "tensorstore/util/str_cat.h"
2522

2623
namespace {
2724

2825
namespace kvstore = tensorstore::kvstore;
2926
using ::tensorstore::MatchesStatus;
3027

28+
TEST(KeyValueStoreTest, OpenInvalidTensorStore) {
29+
auto context = tensorstore::Context::Default();
30+
EXPECT_THAT(
31+
kvstore::Open({{"driver", "json"}}, context).result(),
32+
MatchesStatus(absl::StatusCode::kInvalidArgument,
33+
"Error parsing object member \"driver\": "
34+
"\"json\" is a TensorStore driver, not a KvStore driver"));
35+
}
36+
3137
TEST(KeyValueStoreTest, OpenInvalid) {
3238
auto context = tensorstore::Context::Default();
3339
EXPECT_THAT(kvstore::Open({{"driver", "invalid"}}, context).result(),
3440
MatchesStatus(absl::StatusCode::kInvalidArgument,
3541
"Error parsing object member \"driver\": "
36-
"\"invalid\" is not registered"));
42+
"\"invalid\" is not a registered KvStore driver"));
3743
}
3844

3945
} // namespace

tensorstore/kvstore/registry.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "absl/status/status.h"
3232
#include "tensorstore/context.h"
3333
#include "tensorstore/internal/context_binding.h"
34+
#include "tensorstore/internal/driver_kind_registry.h"
3435
#include "tensorstore/internal/intrusive_ptr.h"
3536
#include "tensorstore/internal/json_registry.h"
3637
#include "tensorstore/json_serialization_options.h"
@@ -244,6 +245,7 @@ template <typename Derived>
244245
class DriverRegistration {
245246
public:
246247
DriverRegistration() {
248+
internal::RegisterDriverKind(Derived::id, internal::DriverKind::kKvStore);
247249
GetDriverRegistry().Register<Derived>(
248250
Derived::id, internal_json_binding::Projection<&Derived::data_>());
249251
serialization::Register<internal::IntrusivePtr<const DriverSpec>,

tensorstore/kvstore/spec.cc

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "absl/status/status.h"
2222
#include <nlohmann/json.hpp>
2323
#include "tensorstore/context.h"
24+
#include "tensorstore/internal/driver_kind_registry.h"
2425
#include "tensorstore/internal/intrusive_ptr.h"
2526
#include "tensorstore/internal/json_binding/bindable.h"
2627
#include "tensorstore/internal/json_binding/json_binding.h"
@@ -30,8 +31,10 @@
3031
#include "tensorstore/serialization/registry.h"
3132
#include "tensorstore/serialization/serialization.h"
3233
#include "tensorstore/util/garbage_collection/garbage_collection.h"
34+
#include "tensorstore/util/quote_string.h"
3335
#include "tensorstore/util/result.h"
3436
#include "tensorstore/util/status.h"
37+
#include "tensorstore/util/str_cat.h"
3538

3639
using ::tensorstore::internal::IntrusivePtr;
3740

@@ -94,7 +97,19 @@ TENSORSTORE_DEFINE_JSON_DEFAULT_BINDER(Spec, [](auto is_loading,
9497
namespace jb = tensorstore::internal_json_binding;
9598
auto& registry = internal_kvstore::GetDriverRegistry();
9699
return jb::NestedContextJsonBinder(jb::Object(
97-
jb::Member("driver", jb::Projection<&Spec::driver>(registry.KeyBinder())),
100+
jb::Member("driver",
101+
jb::Projection<&Spec::driver>(
102+
registry.KeyBinder([](std::string_view unregistered_id) {
103+
auto kind = internal::GetDriverKind(unregistered_id);
104+
if (kind) {
105+
return absl::InvalidArgumentError(tensorstore::StrCat(
106+
tensorstore::QuoteString(unregistered_id),
107+
" is a ", *kind, " driver, not a KvStore driver"));
108+
}
109+
return absl::InvalidArgumentError(tensorstore::StrCat(
110+
tensorstore::QuoteString(unregistered_id),
111+
" is not a registered KvStore driver"));
112+
}))),
98113
jb::Initialize([](Spec* p) {
99114
const_cast<DriverSpec&>(*p->driver).context_binding_state_ =
100115
ContextBindingState::unbound;

0 commit comments

Comments
 (0)