diff --git a/clarifai/runners/pipeline_steps/pipeline_step_builder.py b/clarifai/runners/pipeline_steps/pipeline_step_builder.py index ea7088c5..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 @@ -222,7 +223,27 @@ 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 + 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 diff --git a/tests/runners/test_pipeline_step_builder.py b/tests/runners/test_pipeline_step_builder.py index 146356e1..f6cf303e 100644 --- a/tests/runners/test_pipeline_step_builder.py +++ b/tests/runners/test_pipeline_step_builder.py @@ -217,6 +217,41 @@ 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..1aec48d7 --- /dev/null +++ b/tests/test_requirements.py @@ -0,0 +1,66 @@ +"""Tests for requirements.txt validation.""" + +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}" + )