Skip to content

Commit 63898f1

Browse files
fix: simplify _download_from_gcs parameter list via dataclass
Refactored `_download_from_gcs` to use the `model` description object as a configuration object instead of passing individual fields. This reduces the parameter count and improves maintainability by leveraging existing data structures. Changes: - Updated `_download_from_gcs` signature to `(cls, model: T, cache_dir: str, **kwargs: Any)`. - Updated call site in `download_model`. - Updated corresponding unit tests in `tests/test_model_management.py`. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent 786423e commit 63898f1

2 files changed

Lines changed: 12 additions & 16 deletions

File tree

qwen3_embed/common/model_management.py

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -601,18 +601,18 @@ def _download_from_hf(
601601
@classmethod
602602
def _download_from_gcs(
603603
cls,
604-
model_name: str,
605-
url_source: str | None,
604+
model: T,
606605
cache_dir: str,
607-
deprecated_tar_struct: bool,
608-
local_files_only: bool,
606+
**kwargs: Any,
609607
) -> Path | None:
608+
url_source = model.sources.url
609+
local_files_only = kwargs.get("local_files_only", False)
610610
try:
611611
return cls.retrieve_model_gcs(
612-
model_name,
612+
model.model,
613613
str(url_source),
614614
str(cache_dir),
615-
deprecated_tar_struct=deprecated_tar_struct,
615+
deprecated_tar_struct=model.sources.deprecated_tar_struct,
616616
local_files_only=local_files_only,
617617
)
618618
except (OSError, ValueError, requests.RequestException, tarfile.TarError):
@@ -684,11 +684,9 @@ def download_model(cls, model: T, cache_dir: str, retries: int = 3, **kwargs: An
684684

685685
if url_source or local_files_only:
686686
gcs_path = cls._download_from_gcs(
687-
model_name=model.model,
688-
url_source=url_source,
687+
model=model,
689688
cache_dir=cache_dir,
690-
deprecated_tar_struct=model.sources.deprecated_tar_struct,
691-
local_files_only=local_files_only,
689+
**kwargs,
692690
)
693691
if gcs_path:
694692
return gcs_path

tests/test_model_management.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,11 +1234,10 @@ def test_download_from_gcs_returns_none_on_exception(self, mock_logger, tmp_path
12341234
with patch.object(
12351235
ModelManagement, "retrieve_model_gcs", side_effect=ValueError("GCS Error")
12361236
):
1237+
model = make_model_description(url="http://example.com/model.tar.gz")
12371238
result = ModelManagement._download_from_gcs(
1238-
model_name="test/model",
1239-
url_source="http://example.com/model.tar.gz",
1239+
model=model,
12401240
cache_dir=str(tmp_path),
1241-
deprecated_tar_struct=False,
12421241
local_files_only=False,
12431242
)
12441243

@@ -1253,11 +1252,10 @@ def test_download_from_gcs_no_logger_on_local_files_only(self, mock_logger, tmp_
12531252
with patch.object(
12541253
ModelManagement, "retrieve_model_gcs", side_effect=ValueError("GCS Error")
12551254
):
1255+
model = make_model_description(url="http://example.com/model.tar.gz")
12561256
result = ModelManagement._download_from_gcs(
1257-
model_name="test/model",
1258-
url_source="http://example.com/model.tar.gz",
1257+
model=model,
12591258
cache_dir=str(tmp_path),
1260-
deprecated_tar_struct=False,
12611259
local_files_only=True,
12621260
)
12631261

0 commit comments

Comments
 (0)