Skip to content

[EAGLE-6237]: update the ClarifaiUrlHelper be much easier to use from context #608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 29, 2025

Conversation

zeiler
Copy link
Member

@zeiler zeiler commented May 29, 2025

This pull request refactors the handling of context in the ClarifaiUrlHelper class and its usage across multiple modules. The changes streamline the code by centralizing the retrieval of the current context, enabling default values for user, app, and resource IDs when not explicitly provided. This reduces redundancy and simplifies the API and UI URL generation logic.

Now if you have a valid clarifai config use {context} setup then you can run the following code to get an MCP url for example:

from clarifai.urls.helper import ClarifaiUrlHelper

url = ClarifaiUrlHelper().mcp_api_url() # get url from the current clarifai config

If you want to set the fields manually you can do that still and they take precedence:

from clarifai.urls.helper import ClarifaiUrlHelper

url = ClarifaiUrlHelper().mcp_api_url(user_id, app_id, model_id) # get url from the current clarifai config

So the precedence is prefer args, then prefer env vars like CLARIFAI_USER_ID, etc. then fall back to what the context has.

@zeiler zeiler requested review from Copilot and mogith-pn May 29, 2025 02:58
Copy link
Contributor

@Copilot 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 pull request refactors and simplifies the handling of context in the ClarifaiUrlHelper, allowing the helper to automatically use configuration values when explicit parameters are not provided. Key changes include:

  • Updating initializer logic to derive authentication parameters from the current context when auth is None.
  • Refactoring URL-generating methods to default missing parameters to values from the current context.
  • Simplifying the usage in client and runner modules by eliminating redundant context retrieval.

Reviewed Changes

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

File Description
clarifai/urls/helper.py Refactored methods to use current context defaults and added type hints.
clarifai/runners/utils/code_script.py Removed the explicit use_ctx branch; now always instantiates with context.
clarifai/client/model.py Updated context retrieval for URL generation using the helper.

Copy link

Code Coverage

Package Line Rate Health
clarifai 43%
clarifai.cli 43%
clarifai.client 72%
clarifai.client.auth 74%
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 61%
clarifai.runners.utils 56%
clarifai.runners.utils.data_types 72%
clarifai.schema 100%
clarifai.urls 58%
clarifai.utils 73%
clarifai.utils.evaluation 67%
clarifai.workflows 94%
Summary 65% (6286 / 9673)

Minimum allowed line rate is 50%

Copy link
Contributor

@mogith-pn mogith-pn left a comment

Choose a reason for hiding this comment

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

LGTM.

@zeiler zeiler merged commit fb84831 into master May 29, 2025
9 checks passed
@zeiler zeiler deleted the EAGLE-6237-url-helper-from-ctx branch May 29, 2025 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants