Skip to content

[EAGLE-6237]: update code snippets for MCP/OpenAI and make env vars take precedence #607

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 2 commits into from
May 28, 2025

Conversation

zeiler
Copy link
Member

@zeiler zeiler commented May 28, 2025

This pull request introduces enhancements to the configuration handling, URL generation, and client script generation in the Clarifai codebase. The changes improve flexibility by introducing new default values, methods for constructing URLs for specific endpoints, and updated logic for generating client scripts based on context and method signatures.

Configuration Enhancements:

  • Added support for default fallback values (DEFAULT_UI and DEFAULT_BASE) in the Context class when environment variables are not set. This ensures a more robust configuration handling process.
  • Updated __getattr__ in Context to prioritize environment variables over configuration values and provide detailed error messages for missing attributes.

URL Generation Improvements:

  • Introduced mcp_api_url and openai_api_url methods in ClarifaiUrlHelper to generate URLs for MCP-hosted models and OpenAI-compatible models, respectively. These methods streamline URL creation for specific use cases.

Client Script Generation Updates:

  • Enhanced generate_client_script to support context-based URL generation using Config and ClarifaiUrlHelper. This allows dynamic selection of API endpoints based on the context.
  • Added logic to generate client scripts for OpenAI-compatible models, including an example script for interacting with OpenAI’s API.

Constants and Utilities:

  • Added constants MCP_TRANSPORT_NAME and OPENAI_TRANSPORT_NAME to clarifai/utils/constants.py for identifying specific transport methods.
  • Introduced a utility function has_signature_method to check for the existence of specific method signatures in a list.

These changes collectively enhance the configurability, extensibility, and usability of the Clarifai codebase.

@zeiler zeiler requested a review from Copilot May 28, 2025 17:32
@zeiler zeiler changed the title [EAGLE-6237]: update code snippets for MCP/OpenAI [EAGLE-6237]: update code snippets for MCP/OpenAI and make env vars take precedence May 28, 2025
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 PR enhances configuration defaults, URL generation, and client script templates to support MCP-hosted and OpenAI-compatible models.

  • Adds default UI/base fallbacks in Context and new transport constants.
  • Introduces mcp_api_url and openai_api_url helpers.
  • Updates generate_client_script to branch on transport signatures using context, and updates UI URL construction in model/CLI modules.

Reviewed Changes

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

Show a summary per file
File Description
clarifai/utils/constants.py Added MCP_TRANSPORT_NAME and OPENAI_TRANSPORT_NAME constants
clarifai/utils/config.py Imported DEFAULT_UI/DEFAULT_BASE and extended fallback logic
clarifai/urls/helper.py Added mcp_api_url and openai_api_url methods
clarifai/runners/utils/code_script.py Added has_signature_method, context‐based script generation
clarifai/client/model.py Updated URL template to use current.ui from context
clarifai/cli/model.py Changed log to include full UI URL
Comments suppressed due to low confidence (4)

clarifai/runners/utils/code_script.py:13

  • The List type is used here but not imported; add from typing import List at the top of the file to avoid a NameError.
name: str, method_signatures: List[resources_pb2.MethodSignature]

clarifai/runners/utils/code_script.py:7

  • auth_obj isn't defined or exported from clarifai.urls.helper; ensure you import the correct auth helper (e.g. default_auth) or define auth_obj.
from clarifai.urls.helper import ClarifaiUrlHelper, auth_obj

clarifai/client/model.py:133

  • current.ui already includes the protocol (e.g. "https://"); using f"https://{current.ui}/..." will double the prefix. Remove the hardcoded https:// or normalize current.ui to exclude it.
url = f"https://{current.ui}/{current.user_id}/{current.app_id}/models/{current.model_id}"

clarifai/urls/helper.py:100

  • [nitpick] Fix typos: change "incldue" to "include" and "you an set" to "you can set" for clarity.
It also doesn't incldue the model which you an set as the model arg in an openAI client call

@zeiler zeiler requested a review from ackizilkale May 28, 2025 17:37
Copy link

Code Coverage

Package Line Rate Health
clarifai 43%
clarifai.cli 43%
clarifai.client 71%
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 61%
clarifai.runners.utils.data_types 72%
clarifai.schema 100%
clarifai.urls 71%
clarifai.utils 73%
clarifai.utils.evaluation 67%
clarifai.workflows 94%
Summary 66% (6325 / 9625)

Minimum allowed line rate is 50%

@mogith-pn mogith-pn self-requested a review May 28, 2025 18:52
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 dd35cd1 into master May 28, 2025
9 checks passed
@zeiler zeiler deleted the EAGLE-6237-code-snippets branch May 28, 2025 23:15
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