Skip to content

[PR-716]: Improve Handling for PAT and USER_ID#702

Merged
srikanthbachala20 merged 4 commits intomasterfrom
PR-716-Improve-Handling-for-PAT-and-USER_ID
Jul 15, 2025
Merged

[PR-716]: Improve Handling for PAT and USER_ID#702
srikanthbachala20 merged 4 commits intomasterfrom
PR-716-Improve-Handling-for-PAT-and-USER_ID

Conversation

@srikanthbachala20
Copy link
Copy Markdown
Contributor

This pull request refactors the authentication and logging mechanisms in the Clarifai CLI to improve maintainability and user feedback. The key changes include replacing the validate_pat_token function with validate_context_auth, introducing structured logging via the logger module, and enhancing error handling and user feedback.

Authentication Refactor:

  • Replaced validate_pat_token with validate_context_auth for validating context credentials, consolidating the logic and improving clarity. (clarifai/cli/base.py, clarifai/utils/cli.py) [1] [2]
  • Updated the validate_context_auth function to use structured logging for success and error messages, replacing return values with sys.exit(1) for invalid credentials. (clarifai/utils/cli.py)

Logging Enhancements:

  • Replaced print statements with logger.info and logger.error across the CLI commands to standardize logging and improve debugging. (clarifai/cli/base.py, clarifai/cli/model.py) [1] [2] [3] [4]
  • Added logger import to relevant files for consistent logging usage. (clarifai/utils/cli.py)

Error Handling Improvements:

  • Enhanced error messages for invalid user IDs and missing PATs in the local_runner function, ensuring clear user guidance and proper exits. (clarifai/cli/model.py)
  • Improved error feedback in validate_context_auth for common issues like invalid tokens, SSL errors, and network problems. (clarifai/utils/cli.py)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the Clarifai CLI’s authentication and logging to consolidate credential validation, standardize output via a logger, and improve user feedback and exit behavior.

  • Consolidates PAT validation by replacing validate_pat_token with validate_context_auth and shifting from return values to structured logging and sys.exit.
  • Replaces print statements with logger.info/logger.error across commands for consistent logging.
  • Enhances error handling in local_runner and login/create commands to clearly log missing or invalid credentials before exiting.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
clarifai/utils/cli.py Introduces validate_context_auth with structured logging and exit behavior; deprecates returns
clarifai/cli/model.py Updates local_runner to log and exit on missing user_id or pat; adds display of masked PAT
clarifai/cli/base.py Switches CLI commands to use validate_context_auth and replaces prints with logger calls
Comments suppressed due to low confidence (2)

clarifai/utils/cli.py:179

  • Update the function signature and docstring of validate_context_auth to reflect that it no longer returns a tuple but exits directly. Consider using -> NoReturn and removing or revising the Returns section.
def validate_context_auth(pat: str, user_id: str, api_base: str = None) -> Tuple[bool, str]:

clarifai/cli/model.py:475

  • The logger is used here but not imported in this module; add from clarifai.utils.logging import logger at the top of the file.
    logger.info(f"Current user_id: {user_id}")

Copy link
Copy Markdown
Contributor

@luv-bansal luv-bansal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just small comments related to better message display. Else looks good!

Comment thread clarifai/cli/base.py Outdated
Comment thread clarifai/utils/cli.py Outdated
Copy link
Copy Markdown
Contributor

@luv-bansal luv-bansal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good!

@github-actions
Copy link
Copy Markdown

Code Coverage

Package Line Rate Health
clarifai 43%
clarifai.cli 44%
clarifai.cli.templates 28%
clarifai.client 69%
clarifai.client.auth 66%
clarifai.constants 100%
clarifai.datasets 100%
clarifai.datasets.export 80%
clarifai.datasets.upload 75%
clarifai.datasets.upload.loaders 37%
clarifai.models 100%
clarifai.modules 0%
clarifai.rag 72%
clarifai.runners 12%
clarifai.runners.models 58%
clarifai.runners.pipeline_steps 45%
clarifai.runners.pipelines 85%
clarifai.runners.utils 62%
clarifai.runners.utils.data_types 72%
clarifai.schema 100%
clarifai.urls 58%
clarifai.utils 70%
clarifai.utils.evaluation 67%
clarifai.workflows 95%
Summary 63% (7303 / 11528)

Minimum allowed line rate is 50%

@srikanthbachala20 srikanthbachala20 merged commit 047e703 into master Jul 15, 2025
10 checks passed
@srikanthbachala20 srikanthbachala20 deleted the PR-716-Improve-Handling-for-PAT-and-USER_ID branch July 15, 2025 07:57
@luv-bansal luv-bansal mentioned this pull request Jul 23, 2025
srikanthbachala20 added a commit that referenced this pull request Jul 29, 2025
* [PR-716]: Improve Handling for PAT  and USER_ID

* [PR-716]: Improve Handling for PAT  and USER_ID

* [PR-716]: Improve Handling for PAT  and USER_ID

* [PR-716]: Improve Handling for PAT  and USER_ID
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants