Fix #2524: improve user experience when trying to pull models from Hu...

[JiwaniZakir]

Closes #2524

Surfaces the original HTTP error (e.g. 404) when pulling from Hugging Face instead of masking it with a misleading NotImplementedError.

Changes

ramalama/hf_style_repo_base.py — HFStyleRepoModel pull path

Wrapped the call to self.get_cli_download_args(tempdir, model) in a try/except NotImplementedError block. When get_cli_download_args raises NotImplementedError (the "huggingface cli download not available" path), the code now re-raises e — the original exception from the earlier create_repository attempt — via raise e from None, so the real failure reason reaches the user.

test/unit/test_transport_factory.py — new regression test

Added test_hf_pull_surfaces_http_error_not_notimplementederror, which patches create_repository to raise a KeyError containing a 404 message string, patches available to return True (simulating an installed CLI), and asserts that model.pull() propagates the KeyError rather than swallowing it. Imports MagicMock, PropertyMock, and patch from unittest.mock were added to support the new test.

Motivation

When create_repository failed with an HTTP 404 (model file not found on HF), the exception was stored in e and control fell through to the CLI download path. There, get_cli_download_args raised NotImplementedError with the generic "huggingface cli download not available" message, which replaced the informative HTTP error. Users with huggingface_hub correctly installed still saw the misleading CLI-missing error with no indication that the model path itself was wrong.

Testing

The new unit test covers the regression directly:

pytest test/unit/test_transport_factory.py::test_hf_pull_surfaces_http_error_not_notimplementederror

The test confirms that a KeyError containing "HTTP Error 404: Not Found" propagates out of pull() unchanged, and that NotImplementedError does not surface in its place.

---

This PR was created with AI assistance (Claude). The changes were reviewed by quality gates and a critic model before submission.

Summary by Sourcery

Surface original Hugging Face HTTP errors when model pulls fail instead of masking them with a generic NotImplementedError from the CLI download path.

Bug Fixes:

• Ensure HTTP errors (e.g. 404) from Hugging Face model pulls are propagated to the user rather than replaced by a misleading NotImplementedError.

Tests:

• Add a regression test verifying that Hugging Face pull propagates the original HTTP error (e.g. 404) and does not surface NotImplementedError instead.

[sourcery-ai]

Reviewer's Guide

Adjusts HF-style model pull logic to surface the original HTTP error instead of a misleading NotImplementedError and adds a regression test to lock in the new behavior.

File-Level Changes

Change	Details	Files
Ensure HF-style model pulls re-raise the original repository creation error instead of masking it with a NotImplementedError from the CLI path.	Wrap the get_cli_download_args call in pull() in a try/except NotImplementedError block On NotImplementedError, re-raise the earlier create_repository exception using raise e from None so the original HTTP error reaches the caller Leave the CLI download path unchanged when get_cli_download_args does not raise NotImplementedError	ramalama/hf_style_repo_base.py
Add regression coverage that HF HTTP errors (e.g., 404) propagate out of model.pull() unchanged.	Introduce test_hf_pull_surfaces_http_error_not_notimplementederror to assert that a KeyError raised from create_repository propagates from pull() Patch model_store, create_repository, available(), os.makedirs, and TemporaryDirectory to simulate a HF 404 with an installed CLI and an isolated tempdir Add necessary unittest.mock imports to support the new test	test/unit/test_transport_factory.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#2524	Surface the original Hugging Face HTTP error (e.g., 404) to the user instead of masking it with a misleading 'huggingface cli download not available' NotImplementedError when pulling models.	✅
#2524	Add a regression test to ensure that HTTP errors from the Hugging Face API are propagated and not replaced by the NotImplementedError about the missing CLI.	✅

Possibly linked issues

• improve user experience when trying to pull models from Hugging Face #2524: They match: the PR changes HF pull logic to surface underlying HTTP errors, directly solving the reported UX issue.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• Catching a bare NotImplementedError around get_cli_download_args will also intercept any future NotImplementedError raised inside that code path for unrelated reasons; consider narrowing the exception source (e.g., by moving the try/except closer to where the CLI availability is checked or introducing a more specific custom exception).
• The test’s manual stubbing of TemporaryDirectory.__enter__/__exit__ is a bit brittle; using tempfile.TemporaryDirectory as-is with a real temporary directory, or a small helper/context manager to encapsulate this setup, would make the test easier to follow and less dependent on implementation details.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Catching a bare `NotImplementedError` around `get_cli_download_args` will also intercept any future `NotImplementedError` raised inside that code path for unrelated reasons; consider narrowing the exception source (e.g., by moving the try/except closer to where the CLI availability is checked or introducing a more specific custom exception).
- The test’s manual stubbing of `TemporaryDirectory.__enter__`/`__exit__` is a bit brittle; using `tempfile.TemporaryDirectory` as-is with a real temporary directory, or a small helper/context manager to encapsulate this setup, would make the test easier to follow and less dependent on implementation details.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[gemini-code-assist]

Code Review

This pull request introduces a try-except block in hf_style_repo_base.py to handle NotImplementedError during model pulls and adds a regression test. A bug was introduced where an undefined variable e is raised in the exception handler. Review feedback suggests adding a descriptive error message before re-raising the exception to improve user context and maintain consistency.

[gemini-code-assist]

For consistency with the case where the CLI tool is not available at all (lines 362-364), it would be beneficial to also print an error message here before re-raising the original exception. This would inform the user that a fallback to CLI download was attempted but failed because it's not implemented, providing more context for the failure.

perror(f"URL pull failed and {self.get_cli_command()} download not available")
raise e from None

[rhatdan]

@JiwaniZakir This makes sense? Do you agree?

[github-actions]

A friendly reminder that this PR had no activity for 30 days.