Skip to content

Fix Model upload#606

Merged
luv-bansal merged 3 commits intomasterfrom
fix-model-upload-1
May 28, 2025
Merged

Fix Model upload#606
luv-bansal merged 3 commits intomasterfrom
fix-model-upload-1

Conversation

@luv-bansal
Copy link
Copy Markdown
Contributor

@luv-bansal luv-bansal commented May 28, 2025

Fixing the Model upload!!!

Before getting error

  File "/Users/luvbansal/miniconda3/envs/testing-logs/lib/python3.11/site-packages/clarifai/runners/models/model_builder.py", line 1115, in upload_model
    builder.upload_model_version()
  File "/Users/luvbansal/miniconda3/envs/testing-logs/lib/python3.11/site-packages/clarifai/runners/models/model_builder.py", line 910, in upload_model_version
    model_version_proto = self.get_model_version_proto()
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/luvbansal/miniconda3/envs/testing-logs/lib/python3.11/site-packages/clarifai/runners/models/model_builder.py", line 844, in get_model_version_proto
    model_version_proto = resources_pb2.ModelVersion(
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Parameter to MergeFrom() must be instance of same class: expected <class 'proto.clarifai.api.resources_pb2.MethodSignature'> got <class 'unittest.mock.MagicMock'>.

This PR standardizes the naming of the OpenAI client and model attributes, ensures mock objects are skipped when registering model methods, and publishes a new patch version.

Renamed openai_client ➔ client and model_name ➔ model in both implementation and tests
Enhanced _register_model_methods to filter out MagicMock instances
Bumped package version to 11.4.6 and updated the CHANGELOG

@luv-bansal luv-bansal requested a review from Copilot May 28, 2025 09:42
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 addresses a bug related to model uploads by refactoring how the OpenAI model client and model are accessed.

  • In clarifai/runners/models/openai_class.py, the class attributes have been renamed and adjusted for error handling.
  • In clarifai/runners/models/model_class.py, additional checks have been introduced in the model method registration to skip mocked objects.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
clarifai/runners/models/openai_class.py Renames attributes and updates error handling in the init method
clarifai/runners/models/model_class.py Adds additional filtering for methods, skipping mocked objects

Comment thread clarifai/runners/models/openai_class.py Outdated
@luv-bansal luv-bansal requested a review from Copilot May 28, 2025 09:49
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 standardizes the naming of the OpenAI client and model attributes, ensures mock objects are skipped when registering model methods, and publishes a new patch version.

  • Renamed openai_clientclient and model_namemodel in both implementation and tests
  • Enhanced _register_model_methods to filter out MagicMock instances
  • Bumped package version to 11.4.6 and updated the CHANGELOG

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
clarifai/runners/models/openai_class.py Renamed class attributes and updated __init__ to handle fallback logic
clarifai/runners/models/model_class.py Imported MagicMock and added filtering in _register_model_methods
clarifai/runners/models/dummy_openai_model.py Updated subclass to use client/model attributes
tests/runners/test_openai_model.py Updated tests to reference client instead of openai_client
tests/runners/test_model_classes.py Updated tests to reference client instead of openai_client
tests/openai_model_test.py Updated tests to reference client instead of openai_client
clarifai/init.py Bumped version from 11.4.5 to 11.4.6
CHANGELOG.md Added release notes for version 11.4.6
Comments suppressed due to low confidence (3)

clarifai/runners/models/openai_class.py:30

  • Add a test for the fallback branch where client.models.list() raises an exception, ensuring a NotImplementedError is raised with the correct message.
if self.model is None:

clarifai/runners/models/model_class.py:9

  • [nitpick] Avoid importing unittest.mock in production code; consider relocating mock filtering logic into test utilities or a test-only module.
from unittest.mock import MagicMock

clarifai/runners/models/model_class.py:351

  • [nitpick] The check for classmethod/staticmethod descriptors may not behave as intended; consider using inspect.isfunction or inspect.ismethod to more accurately identify methods.
if not callable(method) or isinstance(method, (classmethod, staticmethod)):

Comment thread CHANGELOG.md
@luv-bansal luv-bansal requested review from mogith-pn and zeiler May 28, 2025 09:53
@github-actions
Copy link
Copy Markdown

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 56%
clarifai.runners.utils.data_types 72%
clarifai.schema 100%
clarifai.urls 75%
clarifai.utils 73%
clarifai.utils.evaluation 67%
clarifai.workflows 94%
Summary 65% (6247 / 9597)

Minimum allowed line rate is 50%

@luv-bansal luv-bansal enabled auto-merge (squash) May 28, 2025 10:00
Copy link
Copy Markdown
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.

@luv-bansal luv-bansal merged commit 77952eb into master May 28, 2025
9 checks passed
@luv-bansal luv-bansal deleted the fix-model-upload-1 branch May 28, 2025 10:08
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