Skip to content

Commit 885923a

Browse files
fix
1 parent 4c0e37d commit 885923a

File tree

4 files changed

+43
-9
lines changed

4 files changed

+43
-9
lines changed

src/cli_parser.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -443,10 +443,16 @@ void CLIParser::prepareGraph(HFSettingsImpl& hfSettings, const std::string& mode
443443
hfSettings.pullHfAndStartModelMode = true;
444444
if (result->count("overwrite_models"))
445445
hfSettings.overwriteModels = result->operator[]("overwrite_models").as<bool>();
446-
if (result->count("source_model"))
446+
if (result->count("source_model")){
447447
hfSettings.sourceModel = result->operator[]("source_model").as<std::string>();
448-
if (result->count("model_repository_path"))
448+
} else {
449+
throw std::logic_error("source_model parameter is required for pull mode");
450+
}
451+
if (result->count("model_repository_path")){
449452
hfSettings.downloadPath = result->operator[]("model_repository_path").as<std::string>();
453+
} else {
454+
throw std::logic_error("model_repository_path parameter is required for pull mode");
455+
}
450456
if (result->count("task")) {
451457
hfSettings.task = stringToEnum(result->operator[]("task").as<std::string>());
452458
switch (hfSettings.task) {

src/config.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,17 @@ bool Config::check_hostname_or_ip(const std::string& input) {
8686
}
8787

8888
bool Config::validate() {
89-
// TODO: CVS-166727 Add validation of all parameters once the CLI model export flags will be implemented
9089
if (this->serverSettings.hfSettings.pullHfModelMode) {
9190
if (this->serverSettings.hfSettings.task == unknown) {
9291
std::cerr << "Error: --task parameter not set." << std::endl;
9392
return false;
9493
}
94+
#if (PYTHON_DISABLE == 1)
95+
if (serverSettings.hfSettings.sourceModel.rfind("OpenVINO/", 0) != 0) {
96+
std::cerr << "For now OVMS version without python supports pulling OpenVINO models only";
97+
return false;
98+
}
99+
#endif
95100
if (this->serverSettings.hfSettings.task == text_generation) {
96101
if (!std::holds_alternative<TextGenGraphSettingsImpl>(this->serverSettings.hfSettings.graphSettings)) {
97102
std::cerr << "Graph options not initialized for text generation.";
@@ -127,11 +132,6 @@ bool Config::validate() {
127132
return false;
128133
}
129134
}
130-
131-
if (serverSettings.hfSettings.sourceModel.rfind("OpenVINO/", 0) != 0) {
132-
std::cerr << "For now only OpenVINO models are supported";
133-
return false;
134-
}
135135
}
136136

137137
if (this->serverSettings.hfSettings.task == embeddings) {

src/graph_export/graph_export.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ static Status createTextGenerationGraphTemplate(const std::string& directoryPath
125125
::mediapipe::CalculatorGraphConfig config;
126126
bool success = ::google::protobuf::TextFormat::ParseFromString(oss.str(), &config);
127127
if (!success) {
128-
SPDLOG_ERROR("Created graph config couldn't be parsed.");
128+
SPDLOG_ERROR("Created graph config couldn't be parsed - some parameters values are invalid.");
129129
return StatusCode::MEDIAPIPE_GRAPH_CONFIG_FILE_INVALID;
130130
}
131131
// clang-format on

src/test/ovmsconfig_test.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,34 @@ TEST_F(OvmsConfigDeathTest, hfBadEmbeddingsGraphNoPort) {
540540
EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "port and rest_port cannot both be unset");
541541
}
542542

543+
TEST_F(OvmsConfigDeathTest, hfPullNoSourceModel) {
544+
char* n_argv[] = {
545+
"ovms",
546+
"--model_repository_path",
547+
"/some/path",
548+
"--task",
549+
"embeddings",
550+
"--normalize",
551+
"true",
552+
};
553+
int arg_count = 7;
554+
EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "port and rest_port cannot both be unset");
555+
}
556+
557+
TEST_F(OvmsConfigDeathTest, hfPullNoSourceModelRepositoryPath) {
558+
char* n_argv[] = {
559+
"ovms",
560+
"--source_model",
561+
"some/model",
562+
"--task",
563+
"embeddings",
564+
"--normalize",
565+
"true",
566+
};
567+
int arg_count = 7;
568+
EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "port and rest_port cannot both be unset");
569+
}
570+
543571
TEST(OvmsGraphConfigTest, positiveAllChanged) {
544572
std::string modelName = "OpenVINO/Phi-3-mini-FastDraft-50M-int8-ov";
545573
std::string downloadPath = "test/repository";

0 commit comments

Comments
 (0)