Skip to content

Commit

Permalink
Deprecate Invalid config checking (#304)
Browse files Browse the repository at this point in the history
Signed-off-by: Li Liu <[email protected]>
  • Loading branch information
liliu-z committed Dec 14, 2023
1 parent 04ce7b1 commit 16c1f7e
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 18 deletions.
31 changes: 16 additions & 15 deletions src/common/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,22 @@ static const std::unordered_set<std::string> ext_legal_json_keys = {"metric_type

Status
Config::FormatAndCheck(const Config& cfg, Json& json, std::string* const err_msg) {
try {
for (auto& it : json.items()) {
// valid only if it.key() exists in one of cfg.__DICT__ and ext_legal_json_keys
if (cfg.__DICT__.find(it.key()) == cfg.__DICT__.end() &&
ext_legal_json_keys.find(it.key()) == ext_legal_json_keys.end()) {
throw KnowhereException(std::string("invalid json key ") + it.key());
}
}
} catch (std::exception& e) {
LOG_KNOWHERE_ERROR_ << e.what();
if (err_msg) {
*err_msg = e.what();
}
return Status::invalid_param_in_json;
}
// Deprecated invalid json key check for now
// try {
// for (auto& it : json.items()) {
// // valid only if it.key() exists in one of cfg.__DICT__ and ext_legal_json_keys
// if (cfg.__DICT__.find(it.key()) == cfg.__DICT__.end() &&
// ext_legal_json_keys.find(it.key()) == ext_legal_json_keys.end()) {
// throw KnowhereException(std::string("invalid json key ") + it.key());
// }
// }
// } catch (std::exception& e) {
// LOG_KNOWHERE_ERROR_ << e.what();
// if (err_msg) {
// *err_msg = e.what();
// }
// return Status::invalid_param_in_json;
// }

try {
for (const auto& it : cfg.__DICT__) {
Expand Down
14 changes: 11 additions & 3 deletions tests/ut/test_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,22 @@ TEST_CASE("Test config json parse", "[config]") {
})",
R"({
"12-s": 19878
})",
})");
knowhere::BaseConfig test_config;
knowhere::Json test_json = knowhere::Json::parse(invalid_json_str);
s = knowhere::Config::FormatAndCheck(test_config, test_json);
CHECK(s == knowhere::Status::success);
}

SECTION("check invalid json values") {
auto invalid_json_str = GENERATE(as<std::string>{},
R"({
"k": "100.12"
})");
knowhere::BaseConfig test_config;
knowhere::Json test_json = knowhere::Json::parse(invalid_json_str);
s = knowhere::Config::FormatAndCheck(test_config, test_json);
CHECK(s != knowhere::Status::success);
CHECK(s == knowhere::Status::invalid_value_in_json);
}

SECTION("Check the json for the specific index") {
Expand Down Expand Up @@ -83,7 +91,7 @@ TEST_CASE("Test config json parse", "[config]") {
})");
knowhere::HnswConfig hnsw_config;
s = knowhere::Config::FormatAndCheck(hnsw_config, large_build_json);
CHECK(s == knowhere::Status::invalid_param_in_json);
CHECK(s == knowhere::Status::success);
knowhere::DiskANNConfig diskann_config;
s = knowhere::Config::FormatAndCheck(diskann_config, large_build_json);
CHECK(s == knowhere::Status::success);
Expand Down

0 comments on commit 16c1f7e

Please sign in to comment.