From e8b610f9a6d06f6ab215cec3b8b34593df45564e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Sep 2025 12:04:25 +0000 Subject: [PATCH 1/4] Initial plan From c06521f3926d1f2cc23e0b7f13e368dffa4510fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Sep 2025 12:13:24 +0000 Subject: [PATCH 2/4] Fix _ensure_clarifai_requirement method and loosen requirements constraints Co-authored-by: mogith-pn <143642606+mogith-pn@users.noreply.github.com> --- .../pipeline_steps/pipeline_step_builder.py | 18 +++++++++++++++++- requirements.txt | 10 +++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/clarifai/runners/pipeline_steps/pipeline_step_builder.py b/clarifai/runners/pipeline_steps/pipeline_step_builder.py index ea7088c5..6ba23bce 100644 --- a/clarifai/runners/pipeline_steps/pipeline_step_builder.py +++ b/clarifai/runners/pipeline_steps/pipeline_step_builder.py @@ -222,7 +222,23 @@ def _ensure_clarifai_requirement(self): requirements = f.readlines() # Check if clarifai is already present - has_clarifai = any('clarifai' in line for line in requirements) + has_clarifai = False + for line in requirements: + line = line.strip() + if not line or line.startswith("#"): + continue + try: + # Use packaging.requirements to properly parse package names + from packaging.requirements import Requirement + req = Requirement(line) + if req.name.lower() == 'clarifai': + has_clarifai = True + break + except Exception: + # Fallback for malformed lines - check if line starts with "clarifai" + if line.lower().startswith('clarifai==') or line.lower().startswith('clarifai>=') or line.lower().startswith('clarifai<') or line.lower().startswith('clarifai '): + has_clarifai = True + break if not has_clarifai: requirements.append(f'clarifai=={CLIENT_VERSION}\n') diff --git a/requirements.txt b/requirements.txt index 97ae6c19..f819f6e4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,9 +10,9 @@ fsspec>=2024.6.1 click>=8.1.7 requests>=2.32.3 aiohttp>=3.10.0 -uv==0.7.12 -ruff==0.11.4 -psutil==7.0.0 +uv>=0.7.12 +ruff>=0.11.4 +psutil>=7.0.0 pygments>=2.19.2 -pydantic_core==2.33.2 -packaging==25.0 +pydantic_core>=2.33.2 +packaging>=25.0 From 6e8a1d03faccd38bd323eb2e8dec22d4544e6b4e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Sep 2025 12:15:33 +0000 Subject: [PATCH 3/4] Add comprehensive tests for requirements constraints and clarifai detection fix Co-authored-by: mogith-pn <143642606+mogith-pn@users.noreply.github.com> --- tests/runners/test_pipeline_step_builder.py | 31 +++++++++++ tests/test_requirements.py | 61 +++++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 tests/test_requirements.py diff --git a/tests/runners/test_pipeline_step_builder.py b/tests/runners/test_pipeline_step_builder.py index 146356e1..c9592ded 100644 --- a/tests/runners/test_pipeline_step_builder.py +++ b/tests/runners/test_pipeline_step_builder.py @@ -217,6 +217,37 @@ def test_ensure_clarifai_requirement_already_present( clarifai_count = content.count("clarifai") assert clarifai_count == 1 + def test_ensure_clarifai_requirement_with_clarifai_grpc( + self, mock_base_client, setup_test_folder + ): + """Test that clarifai is added when clarifai-grpc is present but clarifai is not. + + This test covers the bug where 'clarifai' in 'clarifai-grpc' was incorrectly + detected as the clarifai package itself. + """ + builder = PipelineStepBuilder(setup_test_folder) + + # Add clarifai-grpc and other packages to requirements.txt (realistic scenario) + requirements_path = os.path.join(setup_test_folder, "requirements.txt") + with open(requirements_path, 'w') as f: + f.write("clarifai-grpc>=11.8.2\nclarifai-protocol>=0.0.32\nnumpy>=1.22.0\nrequests>=2.32.0\n") + + # Call the method + builder._ensure_clarifai_requirement() + + # Check that clarifai was added + with open(requirements_path, 'r') as f: + content = f.read() + + # Verify clarifai was added (should contain both clarifai-grpc and clarifai) + assert "clarifai-grpc" in content # Should still have clarifai-grpc + assert "clarifai==" in content # Should have added clarifai + + # Make sure we have exactly one clarifai package (not matching clarifai-grpc) + lines = content.strip().split('\n') + clarifai_exact_lines = [line for line in lines if line.strip().startswith('clarifai==')] + assert len(clarifai_exact_lines) == 1, f"Expected exactly 1 clarifai== line, got {len(clarifai_exact_lines)}" + def test_create_dockerfile(self, mock_base_client, setup_test_folder): """Test Dockerfile creation.""" builder = PipelineStepBuilder(setup_test_folder) diff --git a/tests/test_requirements.py b/tests/test_requirements.py new file mode 100644 index 00000000..b83f976c --- /dev/null +++ b/tests/test_requirements.py @@ -0,0 +1,61 @@ +"""Tests for requirements.txt validation.""" + +import os +from pathlib import Path + + +def test_requirements_loosened_constraints(): + """Test that requirements.txt has loosened constraints for specified packages. + + This test ensures that PR #787 changes are applied correctly, + loosening version constraints from == to >= for specific packages. + """ + # Get the requirements.txt path + repo_root = Path(__file__).parent.parent + requirements_path = repo_root / "requirements.txt" + + assert requirements_path.exists(), "requirements.txt should exist" + + # Read requirements + with open(requirements_path, 'r') as f: + content = f.read() + + # Check that specific packages use >= instead of == + loosened_packages = { + 'uv': '0.7.12', + 'ruff': '0.11.4', + 'psutil': '7.0.0', + 'pydantic_core': '2.33.2', + 'packaging': '25.0' + } + + for package, version in loosened_packages.items(): + # Should have >= constraint + expected_constraint = f"{package}>={version}" + assert expected_constraint in content, f"Expected {expected_constraint} in requirements.txt" + + # Should NOT have == constraint + forbidden_constraint = f"{package}=={version}" + assert forbidden_constraint not in content, f"Should not have exact constraint {forbidden_constraint}" + + # Check that schema is still pinned (as mentioned in PR description) + assert "schema==0.7.5" in content, "schema should remain pinned to ==0.7.5" + + +def test_requirements_file_format(): + """Test that requirements.txt is properly formatted.""" + repo_root = Path(__file__).parent.parent + requirements_path = repo_root / "requirements.txt" + + with open(requirements_path, 'r') as f: + lines = f.readlines() + + # Check that there are no duplicate blank lines at the end + non_empty_lines = [line for line in lines if line.strip()] + assert len(lines) - len(non_empty_lines) <= 1, "Should have at most one trailing newline" + + # Check that all lines are properly formatted (no leading/trailing whitespace issues) + for i, line in enumerate(lines): + if line.strip(): # Skip empty lines + assert not line.startswith(' '), f"Line {i+1} should not start with space: {line!r}" + assert not line.endswith(' \n'), f"Line {i+1} should not have trailing space: {line!r}" \ No newline at end of file From 60de250bb1f3435ee37f312ce0106cb478b25c1f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Sep 2025 12:38:46 +0000 Subject: [PATCH 4/4] Fix linting issues: move import to top-level and format code properly Co-authored-by: mogith-pn <143642606+mogith-pn@users.noreply.github.com> --- .../pipeline_steps/pipeline_step_builder.py | 9 ++++- tests/runners/test_pipeline_step_builder.py | 12 ++++-- tests/test_requirements.py | 39 +++++++++++-------- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/clarifai/runners/pipeline_steps/pipeline_step_builder.py b/clarifai/runners/pipeline_steps/pipeline_step_builder.py index 6ba23bce..5979114d 100644 --- a/clarifai/runners/pipeline_steps/pipeline_step_builder.py +++ b/clarifai/runners/pipeline_steps/pipeline_step_builder.py @@ -9,6 +9,7 @@ from clarifai_grpc.grpc.api import resources_pb2, service_pb2 from clarifai_grpc.grpc.api.status import status_code_pb2 from google.protobuf import json_format +from packaging.requirements import Requirement from clarifai.client.base import BaseClient from clarifai.utils.logging import logger @@ -229,14 +230,18 @@ def _ensure_clarifai_requirement(self): continue try: # Use packaging.requirements to properly parse package names - from packaging.requirements import Requirement req = Requirement(line) if req.name.lower() == 'clarifai': has_clarifai = True break except Exception: # Fallback for malformed lines - check if line starts with "clarifai" - if line.lower().startswith('clarifai==') or line.lower().startswith('clarifai>=') or line.lower().startswith('clarifai<') or line.lower().startswith('clarifai '): + if ( + line.lower().startswith('clarifai==') + or line.lower().startswith('clarifai>=') + or line.lower().startswith('clarifai<') + or line.lower().startswith('clarifai ') + ): has_clarifai = True break diff --git a/tests/runners/test_pipeline_step_builder.py b/tests/runners/test_pipeline_step_builder.py index c9592ded..f6cf303e 100644 --- a/tests/runners/test_pipeline_step_builder.py +++ b/tests/runners/test_pipeline_step_builder.py @@ -221,7 +221,7 @@ def test_ensure_clarifai_requirement_with_clarifai_grpc( self, mock_base_client, setup_test_folder ): """Test that clarifai is added when clarifai-grpc is present but clarifai is not. - + This test covers the bug where 'clarifai' in 'clarifai-grpc' was incorrectly detected as the clarifai package itself. """ @@ -230,7 +230,9 @@ def test_ensure_clarifai_requirement_with_clarifai_grpc( # Add clarifai-grpc and other packages to requirements.txt (realistic scenario) requirements_path = os.path.join(setup_test_folder, "requirements.txt") with open(requirements_path, 'w') as f: - f.write("clarifai-grpc>=11.8.2\nclarifai-protocol>=0.0.32\nnumpy>=1.22.0\nrequests>=2.32.0\n") + f.write( + "clarifai-grpc>=11.8.2\nclarifai-protocol>=0.0.32\nnumpy>=1.22.0\nrequests>=2.32.0\n" + ) # Call the method builder._ensure_clarifai_requirement() @@ -242,11 +244,13 @@ def test_ensure_clarifai_requirement_with_clarifai_grpc( # Verify clarifai was added (should contain both clarifai-grpc and clarifai) assert "clarifai-grpc" in content # Should still have clarifai-grpc assert "clarifai==" in content # Should have added clarifai - + # Make sure we have exactly one clarifai package (not matching clarifai-grpc) lines = content.strip().split('\n') clarifai_exact_lines = [line for line in lines if line.strip().startswith('clarifai==')] - assert len(clarifai_exact_lines) == 1, f"Expected exactly 1 clarifai== line, got {len(clarifai_exact_lines)}" + assert len(clarifai_exact_lines) == 1, ( + f"Expected exactly 1 clarifai== line, got {len(clarifai_exact_lines)}" + ) def test_create_dockerfile(self, mock_base_client, setup_test_folder): """Test Dockerfile creation.""" diff --git a/tests/test_requirements.py b/tests/test_requirements.py index b83f976c..1aec48d7 100644 --- a/tests/test_requirements.py +++ b/tests/test_requirements.py @@ -1,43 +1,46 @@ """Tests for requirements.txt validation.""" -import os from pathlib import Path def test_requirements_loosened_constraints(): """Test that requirements.txt has loosened constraints for specified packages. - + This test ensures that PR #787 changes are applied correctly, loosening version constraints from == to >= for specific packages. """ # Get the requirements.txt path repo_root = Path(__file__).parent.parent requirements_path = repo_root / "requirements.txt" - + assert requirements_path.exists(), "requirements.txt should exist" - + # Read requirements with open(requirements_path, 'r') as f: content = f.read() - + # Check that specific packages use >= instead of == loosened_packages = { 'uv': '0.7.12', - 'ruff': '0.11.4', + 'ruff': '0.11.4', 'psutil': '7.0.0', 'pydantic_core': '2.33.2', - 'packaging': '25.0' + 'packaging': '25.0', } - + for package, version in loosened_packages.items(): # Should have >= constraint expected_constraint = f"{package}>={version}" - assert expected_constraint in content, f"Expected {expected_constraint} in requirements.txt" - + assert expected_constraint in content, ( + f"Expected {expected_constraint} in requirements.txt" + ) + # Should NOT have == constraint forbidden_constraint = f"{package}=={version}" - assert forbidden_constraint not in content, f"Should not have exact constraint {forbidden_constraint}" - + assert forbidden_constraint not in content, ( + f"Should not have exact constraint {forbidden_constraint}" + ) + # Check that schema is still pinned (as mentioned in PR description) assert "schema==0.7.5" in content, "schema should remain pinned to ==0.7.5" @@ -46,16 +49,18 @@ def test_requirements_file_format(): """Test that requirements.txt is properly formatted.""" repo_root = Path(__file__).parent.parent requirements_path = repo_root / "requirements.txt" - + with open(requirements_path, 'r') as f: lines = f.readlines() - + # Check that there are no duplicate blank lines at the end non_empty_lines = [line for line in lines if line.strip()] assert len(lines) - len(non_empty_lines) <= 1, "Should have at most one trailing newline" - + # Check that all lines are properly formatted (no leading/trailing whitespace issues) for i, line in enumerate(lines): if line.strip(): # Skip empty lines - assert not line.startswith(' '), f"Line {i+1} should not start with space: {line!r}" - assert not line.endswith(' \n'), f"Line {i+1} should not have trailing space: {line!r}" \ No newline at end of file + assert not line.startswith(' '), f"Line {i + 1} should not start with space: {line!r}" + assert not line.endswith(' \n'), ( + f"Line {i + 1} should not have trailing space: {line!r}" + )