Fix handling of ramalama login huggingface

rhatdan commented 6 days ago

Fixes: https://github.com/containers/ramalama/issues/465

Summary by Sourcery

Fix the handling of HuggingFace login by refining command execution and model path logic, enhance code clarity with refactored methods, and update garbage collection to include more repository paths.

Bug Fixes:

Fix the handling of login for HuggingFace models by adjusting the command execution and model path logic.

Enhancements:

Refactor the model path and existence check methods to improve code clarity and maintainability.
Update the garbage collection logic to include additional repository paths such as 'file', 'http', and 'https'.
Standardize the model type naming to lowercase for consistency across the codebase.

sourcery-ai[bot] commented 6 days ago

Reviewer's Guide by Sourcery

This PR improves the handling of model paths and HuggingFace login functionality in RamaLama. The changes consolidate model path handling into the base class, standardize model type naming, and improve symlink management. The PR also enhances the login functionality for HuggingFace and updates model type handling.

Updated class diagram for RamaLama model handling

classDiagram
    class BaseModel {
        +login(args)
        +logout(args)
        +pull(args)
        +remove(args)
        +garbage_collection(args)
        +path(args)
        +model_path(args)
        +exists(args)
        +check_valid_model_path(relative_target_path, model_path)
    }
    class Huggingface {
        +login(args)
        +logout(args)
        +pull(args)
        +push(source, args)
        +exec(cmd_args, args)
    }
    BaseModel <|-- Huggingface
    note for BaseModel "Consolidated model path handling and symlink management"
    note for Huggingface "Enhanced login functionality and updated exec method"

File-Level Changes

Change	Details	Files
Refactored model path handling by moving common functionality to the base class	Moved path(), model_path(), exists(), and check_valid_model_path() methods to the base class Removed duplicate implementations from child classes Standardized model path handling across different model types	`ramalama/model.py` `ramalama/huggingface.py`
Improved HuggingFace integration and model type handling	Standardized HuggingFace type name from 'HuggingFace' to 'huggingface' Added 'hf' as a recognized model type Updated model type prefix checking in New() function Fixed HuggingFace CLI command execution by adding missing args parameter	`ramalama/huggingface.py` `ramalama/cli.py`
Enhanced symlink management in model operations	Replaced run_cmd calls with direct Python pathlib and os operations for symlink management Added missing_ok parameter to handle non-existent symlinks safely Improved symlink creation and updating logic	`ramalama/huggingface.py`
Updated repository paths and model type handling	Added 'file', 'http', and 'https' to repository paths Created a centralized model_types list Simplified model removal logic by consolidating error handling	`ramalama/model.py`

Possibly linked issues

#465: The PR fixes the login handling for huggingface, which is the issue described.

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. - **Generate a pull request title:** Write `@sourcery-ai` anywhere in the pull request title to generate a 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. You can also use this command to specify where the summary should be inserted. #### Customizing Your Experience Access your [dashboard](https://app.sourcery.ai) 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](mailto:support@sourcery.ai) for questions or feedback. - Visit our [documentation](https://docs.sourcery.ai) for detailed guides and information. - Keep in touch with the Sourcery team by following us on [X/Twitter](https://x.com/SourceryAI), [LinkedIn](https://www.linkedin.com/company/sourcery-ai/) or [GitHub](https://github.com/sourcery-ai).

rhatdan commented 6 days ago

@bentito PTAL

rhatdan commented 6 days ago

@bmahabirbu @cooktheryan PTAL

bentito commented 6 days ago

@bentito PTAL

do you @rhatdan happen to know the pipx command to checkout the pr branch? I can't quite seem to get it right...

rhatdan commented 6 days ago

No idea, how to do this.

I would do a git clone of the repo and then run ./bin/ramalama

rhatdan commented 6 days ago

git clone git@github.com:containers/ramalama.git
cd ramalama
wget https://github.com/containers/ramalama/pull/467.patch
git am 467.patch
./bin/ramalama login --TOKEN XXXXX huggingface

bmahabirbu commented 6 days ago

@bmahabirbu @cooktheryan PTAL

Did a quick test and worked for me!

./bin/ramalama login --token=XXX huggingface
The token has not been saved to the git credentials helper. Pass `add_to_git_credential=True` in this function directly or `--add-to-git-credential` if using via `huggingface-cli` if you want to set the git credential as well.
Token is valid (permission: fineGrained).
The token `PC` has been saved to /home/bmahabir/.cache/huggingface/stored_tokens
Your token has been saved to /home/bmahabir/.cache/huggingface/token
Login successful.
The current active token is: `PC`

bentito commented 5 days ago

I was able to login huggingface thanks!

bentito commented 5 days ago

I am still getting 401 unauthorized when trying to pull from huggingface after successful login:

ramalama login --token=$HF_TOKEN huggingface
Token will not been saved to git credential helper. Pass `add_to_git_credential=True` if you want to set the git credential as well.
Token is valid (permission: read).
Your token has been saved to /Users/btofel/.cache/huggingface/token
Login successful

what's the part about: add_to_git_credential=True how would I pass that in the command?

Is that related to my fail?:

$ ramalama pull huggingface://Qwen/Qwen2.5-Coder-32B-Instruct
Error: failed to pull https://huggingface.co/Qwen/raw/main/Qwen2.5-Coder-32B-Instruct: HTTP Error 401: Unauthorized

bentito commented 5 days ago

Tried:

$ ramalama pull --authfile /Users/btofel/.cache/huggingface/token huggingface://ibm-granite/granite-3.0-8b-instruct
Error: failed to pull https://huggingface.co/ibm-granite/raw/main/granite-3.0-8b-instruct: HTTP Error 401: Unauthorized

$ ramalama pull --authfile /Users/btofel/.cache/huggingface/foo huggingface://ibm-granite/granite-3.0-8b-instruct
Error: failed to pull https://huggingface.co/ibm-granite/raw/main/granite-3.0-8b-instruct: HTTP Error 401: Unauthorized

if the token is supposed to be referenced as the authfile, I'm not sure it's checking that b/c it doesn't seem to care if it doesn't exist.

rhatdan commented 5 days ago

ramalama login --token=$HF_TOKEN huggingface
Token will not been saved to git credential helper. Pass `add_to_git_credential=True` if you want to set the git credential as well.
Token is valid (permission: read).
Your token has been saved to /Users/btofel/.cache/huggingface/token
Login successful

Do you think we should do this by default?

If you just look at the login command with ramalama --debug login ...

It should show the command ramalama is executing, you could just take that command and execute it locally to see if this fixes your problem.

rhatdan commented 5 days ago

@bentito I have never worked with the hugginface login to pull images, so I don't know if this works, but I would love to work with you to make it work. Could you ping me on the matrix channel.

containers / ramalama