[PR-745]: Improved UX of overall Model CLI #738
Conversation
srikanthbachala20
requested review from
Copilot,
mogith-pn and
luv-bansal
and removed request for
Copilot
There was a problem hiding this comment.
Pull Request Overview
This pull request improves the UX of the Model CLI by enhancing user feedback during model initialization and checkpoint downloads. The changes focus on providing better visibility into download operations and improving code maintainability through refactoring.
- Enhanced GitHub-based model initialization with file preview and interactive prompts
- Improved Hugging Face checkpoint download logging with size information and progress messages
- Code refactoring for better maintainability and clearer parameter passing
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| clarifai/utils/misc.py | Added recursive file listing function for GitHub repositories |
| clarifai/runners/utils/loader.py | Enhanced checkpoint download logging and refactored file listing logic |
| clarifai/runners/server.py | Updated function call to use named parameters for clarity |
| clarifai/runners/models/model_builder.py | Improved success message to include download path |
| clarifai/cli/model.py | Enhanced GitHub initialization workflow with file preview and user prompts |
| clarifai/cli/base.py | Fixed environment variable reference in command construction |
| depth=(depth - 1), | ||
| ) | ||
| else: | ||
| files_to_download.append(point['path']) |
There was a problem hiding this comment.
Files are being added to the list regardless of their type when depth is 0 or when the item is not a directory. This will include directories in the files list when depth reaches 0, which could lead to incorrect file listings.
| files_to_download.append(point['path']) | |
| if point['type'] == 'file': | |
| files_to_download.append(point['path']) |
Copilot uses AI. Check for mistakes.
| @@ -278,8 +278,8 @@ def view(ctx, output_format): | |||
| def run(ctx, script, context=None): | |||
| """Execute a script with the current context's environment""" | |||
| context = ctx.obj.current if not context else context | |||
| cmd = f'CLARIFAI_USER_ID={context.user_id} CLARIFAI_API_BASE={context.base_url} CLARIFAI_PAT={context.pat} ' | |||
| cmd += ' '.join([f'{k}={v}' for k, v in context.env.items()]) | |||
| cmd = f'CLARIFAI_USER_ID={context.user_id} CLARIFAI_API_BASE={context.api_base} CLARIFAI_PAT={context.pat} ' | |||
There was a problem hiding this comment.
The change from context.base_url to context.api_base may be incorrect if the context object doesn't have an api_base attribute. This could cause an AttributeError at runtime.
| cmd = f'CLARIFAI_USER_ID={context.user_id} CLARIFAI_API_BASE={context.api_base} CLARIFAI_PAT={context.pat} ' | |
| api_base = getattr(context, "api_base", getattr(context, "base_url", "")) | |
| cmd = f'CLARIFAI_USER_ID={context.user_id} CLARIFAI_API_BASE={api_base} CLARIFAI_PAT={context.pat} ' |
Copilot uses AI. Check for mistakes.
Minimum allowed line rate is |
This pull request introduces several improvements and refactors to the model initialization and checkpoint download workflows, focusing on better logging, user interaction, and code maintainability. The main changes include enhanced feedback during GitHub-based model initialization, improved reporting of files to be downloaded, more informative logging during Hugging Face checkpoint downloads, and code refactoring for file listing and validation logic.
Model Initialization and GitHub Download Workflow:
clarifai/cli/model.py, including a preview of files to be downloaded and next-step instructions for the user. [1] [2] [3]get_list_of_files_to_downloadutility inclarifai/utils/misc.pyto recursively list files in a GitHub folder, improving transparency about which files will be downloaded.clarifai/cli/model.pyto include the new utility function.Hugging Face Checkpoint Download Improvements:
clarifai/runners/utils/loader.pyto show the total download size (in MB) and a clear message before downloading model checkpoints.get_repo_files_list), and updated the download size calculation to allow filtering by specific files. [1] [2] [3] [4]General Logging and Minor Fixes:
clarifai/runners/models/model_builder.py.clarifai/cli/model.py).servefunction inclarifai/runners/server.pyfor clarity and maintainability.clarifai/cli/base.pyfor consistency.