Add Git validations for publishing tools (#1381)

This commit prevents tools from being published if the underlying Git
repository is unsynced with origin.
This commit is contained in:
Vini Brasil
2024-10-02 11:46:18 -03:00
committed by GitHub
parent 8aba99a67d
commit 7c1f88cd07
9 changed files with 235 additions and 53 deletions

27
poetry.lock generated
View File

@@ -1581,12 +1581,12 @@ files = [
google-auth = ">=2.14.1,<3.0.dev0" google-auth = ">=2.14.1,<3.0.dev0"
googleapis-common-protos = ">=1.56.2,<2.0.dev0" googleapis-common-protos = ">=1.56.2,<2.0.dev0"
grpcio = [ grpcio = [
{version = ">=1.49.1,<2.0dev", optional = true, markers = "python_version >= \"3.11\" and extra == \"grpc\""},
{version = ">=1.33.2,<2.0dev", optional = true, markers = "python_version < \"3.11\" and extra == \"grpc\""}, {version = ">=1.33.2,<2.0dev", optional = true, markers = "python_version < \"3.11\" and extra == \"grpc\""},
{version = ">=1.49.1,<2.0dev", optional = true, markers = "python_version >= \"3.11\" and extra == \"grpc\""},
] ]
grpcio-status = [ grpcio-status = [
{version = ">=1.49.1,<2.0.dev0", optional = true, markers = "python_version >= \"3.11\" and extra == \"grpc\""},
{version = ">=1.33.2,<2.0.dev0", optional = true, markers = "python_version < \"3.11\" and extra == \"grpc\""}, {version = ">=1.33.2,<2.0.dev0", optional = true, markers = "python_version < \"3.11\" and extra == \"grpc\""},
{version = ">=1.49.1,<2.0.dev0", optional = true, markers = "python_version >= \"3.11\" and extra == \"grpc\""},
] ]
proto-plus = ">=1.22.3,<2.0.0dev" proto-plus = ">=1.22.3,<2.0.0dev"
protobuf = ">=3.19.5,<3.20.0 || >3.20.0,<3.20.1 || >3.20.1,<4.21.0 || >4.21.0,<4.21.1 || >4.21.1,<4.21.2 || >4.21.2,<4.21.3 || >4.21.3,<4.21.4 || >4.21.4,<4.21.5 || >4.21.5,<6.0.0.dev0" protobuf = ">=3.19.5,<3.20.0 || >3.20.0,<3.20.1 || >3.20.1,<4.21.0 || >4.21.0,<4.21.1 || >4.21.1,<4.21.2 || >4.21.2,<4.21.3 || >4.21.3,<4.21.4 || >4.21.4,<4.21.5 || >4.21.5,<6.0.0.dev0"
@@ -4266,8 +4266,8 @@ files = [
[package.dependencies] [package.dependencies]
numpy = [ numpy = [
{version = ">=1.23.2", markers = "python_version == \"3.11\""},
{version = ">=1.22.4", markers = "python_version < \"3.11\""}, {version = ">=1.22.4", markers = "python_version < \"3.11\""},
{version = ">=1.23.2", markers = "python_version == \"3.11\""},
{version = ">=1.26.0", markers = "python_version >= \"3.12\""}, {version = ">=1.26.0", markers = "python_version >= \"3.12\""},
] ]
python-dateutil = ">=2.8.2" python-dateutil = ">=2.8.2"
@@ -5212,6 +5212,25 @@ pytest = ">=7.0.0,<9"
docs = ["sphinx (>=5.3)", "sphinx-rtd-theme (>=1.0)"] docs = ["sphinx (>=5.3)", "sphinx-rtd-theme (>=1.0)"]
testing = ["coverage (>=6.2)", "hypothesis (>=5.7.1)"] testing = ["coverage (>=6.2)", "hypothesis (>=5.7.1)"]
[[package]]
name = "pytest-subprocess"
version = "1.5.2"
description = "A plugin to fake subprocess for pytest"
optional = false
python-versions = ">=3.6"
files = [
{file = "pytest_subprocess-1.5.2-py3-none-any.whl", hash = "sha256:23ac7732aa8bd45f1757265b1316eb72a7f55b41fb21e2ca22e149ba3629fa46"},
{file = "pytest_subprocess-1.5.2.tar.gz", hash = "sha256:ad3ca8a35e798bf9c82d9f16d88700b30d98c5a28236117b86c5d6e581a8ed97"},
]
[package.dependencies]
pytest = ">=4.0.0"
[package.extras]
dev = ["changelogd", "nox"]
docs = ["changelogd", "furo", "sphinx", "sphinx-autodoc-typehints", "sphinxcontrib-napoleon"]
test = ["Pygments (>=2.0)", "anyio", "coverage", "docutils (>=0.12)", "pytest (>=4.0)", "pytest-asyncio (>=0.15.1)", "pytest-rerunfailures", "pytest-timeout"]
[[package]] [[package]]
name = "pytest-vcr" name = "pytest-vcr"
version = "1.0.2" version = "1.0.2"
@@ -7331,4 +7350,4 @@ tools = ["crewai-tools"]
[metadata] [metadata]
lock-version = "2.0" lock-version = "2.0"
python-versions = ">=3.10,<=3.13" python-versions = ">=3.10,<=3.13"
content-hash = "b008b28add072e8f002aa90d230b20027f0ecffcd3c4b3fe4ec954f5ac7c46ca" content-hash = "0b833460455da407e9b73fe545bb8dfccc114384d3ce1aa66a1d2fc83afb00f7"

View File

@@ -57,6 +57,7 @@ pytest = "^8.0.0"
pytest-vcr = "^1.0.2" pytest-vcr = "^1.0.2"
python-dotenv = "1.0.0" python-dotenv = "1.0.0"
pytest-asyncio = "^0.23.7" pytest-asyncio = "^0.23.7"
pytest-subprocess = "^1.5.2"
[tool.poetry.scripts] [tool.poetry.scripts]
crewai = "crewai.cli.cli:crewai" crewai = "crewai.cli.cli:crewai"

View File

@@ -2,12 +2,9 @@ from typing import Any, Dict, List, Optional
from rich.console import Console from rich.console import Console
from crewai.cli import git
from crewai.cli.command import BaseCommand, PlusAPIMixin from crewai.cli.command import BaseCommand, PlusAPIMixin
from crewai.cli.utils import ( from crewai.cli.utils import fetch_and_json_env_file, get_project_name
fetch_and_json_env_file,
get_git_remote_url,
get_project_name,
)
console = Console() console = Console()
@@ -91,7 +88,11 @@ class DeployCommand(BaseCommand, PlusAPIMixin):
) )
console.print("Creating deployment...", style="bold blue") console.print("Creating deployment...", style="bold blue")
env_vars = fetch_and_json_env_file() env_vars = fetch_and_json_env_file()
remote_repo_url = get_git_remote_url()
try:
remote_repo_url = git.Repository().origin_url()
except ValueError:
remote_repo_url = None
if remote_repo_url is None: if remote_repo_url is None:
console.print("No remote repository URL found.", style="bold red") console.print("No remote repository URL found.", style="bold red")

80
src/crewai/cli/git.py Normal file
View File

@@ -0,0 +1,80 @@
import subprocess
class Repository:
def __init__(self, path="."):
self.path = path
if not self.is_git_installed():
raise ValueError("Git is not installed or not found in your PATH.")
if not self.is_git_repo():
raise ValueError(f"{self.path} is not a Git repository.")
self.fetch()
def is_git_installed(self) -> bool:
"""Check if Git is installed and available in the system."""
try:
subprocess.run(
["git", "--version"], capture_output=True, check=True, text=True
)
return True
except (subprocess.CalledProcessError, FileNotFoundError):
return False
def fetch(self) -> None:
"""Fetch latest updates from the remote."""
subprocess.run(["git", "fetch"], cwd=self.path, check=True)
def status(self) -> str:
"""Get the git status in porcelain format."""
return subprocess.check_output(
["git", "status", "--branch", "--porcelain"],
cwd=self.path,
encoding="utf-8",
).strip()
def is_git_repo(self) -> bool:
"""Check if the current directory is a git repository."""
try:
subprocess.check_output(
["git", "rev-parse", "--is-inside-work-tree"],
cwd=self.path,
encoding="utf-8",
)
return True
except subprocess.CalledProcessError:
return False
def has_uncommitted_changes(self) -> bool:
"""Check if the repository has uncommitted changes."""
return len(self.status().splitlines()) > 1
def is_ahead_or_behind(self) -> bool:
"""Check if the repository is ahead or behind the remote."""
for line in self.status().splitlines():
if line.startswith("##") and ("ahead" in line or "behind" in line):
return True
return False
def is_synced(self) -> bool:
"""Return True if the Git repository is fully synced with the remote, False otherwise."""
if self.has_uncommitted_changes() or self.is_ahead_or_behind():
return False
else:
return True
def origin_url(self) -> str | None:
"""Get the Git repository's remote URL."""
try:
result = subprocess.run(
["git", "remote", "get-url", "origin"],
cwd=self.path,
capture_output=True,
text=True,
check=True,
)
return result.stdout.strip()
except subprocess.CalledProcessError:
return None

View File

@@ -6,6 +6,7 @@ import subprocess
import tempfile import tempfile
from crewai.cli.command import BaseCommand, PlusAPIMixin from crewai.cli.command import BaseCommand, PlusAPIMixin
from crewai.cli import git
from crewai.cli.utils import ( from crewai.cli.utils import (
get_project_name, get_project_name,
get_project_description, get_project_description,
@@ -59,6 +60,17 @@ class ToolCommand(BaseCommand, PlusAPIMixin):
os.chdir(old_directory) os.chdir(old_directory)
def publish(self, is_public: bool): def publish(self, is_public: bool):
if not git.Repository().is_synced():
console.print(
"[bold red]Failed to publish tool.[/bold red]\n"
"Local changes need to be resolved before publishing. Please do the following:\n"
"* [bold]Commit[/bold] your changes.\n"
"* [bold]Push[/bold] to sync with the remote.\n"
"* [bold]Pull[/bold] the latest changes from the remote.\n"
"\nOnce your repository is up-to-date, retry publishing the tool."
)
raise SystemExit()
project_name = get_project_name(require=True) project_name = get_project_name(require=True)
assert isinstance(project_name, str) assert isinstance(project_name, str)

View File

@@ -1,8 +1,6 @@
import os import os
import shutil import shutil
import click import click
import re
import subprocess
import sys import sys
import importlib.metadata import importlib.metadata
@@ -61,38 +59,6 @@ def parse_toml(content):
return simple_toml_parser(content) return simple_toml_parser(content)
def get_git_remote_url() -> str | None:
"""Get the Git repository's remote URL."""
try:
# Run the git remote -v command
result = subprocess.run(
["git", "remote", "-v"], capture_output=True, text=True, check=True
)
# Get the output
output = result.stdout
# Parse the output to find the origin URL
matches = re.findall(r"origin\s+(.*?)\s+\(fetch\)", output)
if matches:
return matches[0] # Return the first match (origin URL)
else:
console.print("No origin remote found.", style="bold red")
except subprocess.CalledProcessError as e:
console.print(
f"Error running trying to fetch the Git Repository: {e}", style="bold red"
)
except FileNotFoundError:
console.print(
"Git command not found. Make sure Git is installed and in your PATH.",
style="bold red",
)
return None
def get_project_name( def get_project_name(
pyproject_path: str = "pyproject.toml", require: bool = False pyproject_path: str = "pyproject.toml", require: bool = False
) -> str | None: ) -> str | None:

View File

@@ -143,11 +143,11 @@ class TestDeployCommand(unittest.TestCase):
mock_display.assert_called_once_with({"uuid": "test-uuid"}) mock_display.assert_called_once_with({"uuid": "test-uuid"})
@patch("crewai.cli.deploy.main.fetch_and_json_env_file") @patch("crewai.cli.deploy.main.fetch_and_json_env_file")
@patch("crewai.cli.deploy.main.get_git_remote_url") @patch("crewai.cli.deploy.main.git.Repository.origin_url")
@patch("builtins.input") @patch("builtins.input")
def test_create_crew(self, mock_input, mock_get_git_remote_url, mock_fetch_env): def test_create_crew(self, mock_input, mock_git_origin_url, mock_fetch_env):
mock_fetch_env.return_value = {"ENV_VAR": "value"} mock_fetch_env.return_value = {"ENV_VAR": "value"}
mock_get_git_remote_url.return_value = "https://github.com/test/repo.git" mock_git_origin_url.return_value = "https://github.com/test/repo.git"
mock_input.return_value = "" mock_input.return_value = ""
mock_response = MagicMock() mock_response = MagicMock()

101
tests/cli/test_git.py Normal file
View File

@@ -0,0 +1,101 @@
from crewai.cli.git import Repository
import pytest
@pytest.fixture()
def repository(fp):
fp.register(["git", "--version"], stdout="git version 2.30.0\n")
fp.register(["git", "rev-parse", "--is-inside-work-tree"], stdout="true\n")
fp.register(["git", "fetch"], stdout="")
return Repository(path=".")
def test_init_with_invalid_git_repo(fp):
fp.register(["git", "--version"], stdout="git version 2.30.0\n")
fp.register(
["git", "rev-parse", "--is-inside-work-tree"],
returncode=1,
stderr="fatal: not a git repository\n",
)
with pytest.raises(ValueError):
Repository(path="invalid/path")
def test_is_git_not_installed(fp):
fp.register(["git", "--version"], returncode=1)
with pytest.raises(
ValueError, match="Git is not installed or not found in your PATH."
):
Repository(path=".")
def test_status(fp, repository):
fp.register(
["git", "status", "--branch", "--porcelain"],
stdout="## main...origin/main [ahead 1]\n",
)
assert repository.status() == "## main...origin/main [ahead 1]"
def test_has_uncommitted_changes(fp, repository):
fp.register(
["git", "status", "--branch", "--porcelain"],
stdout="## main...origin/main\n M somefile.txt\n",
)
assert repository.has_uncommitted_changes() is True
def test_is_ahead_or_behind(fp, repository):
fp.register(
["git", "status", "--branch", "--porcelain"],
stdout="## main...origin/main [ahead 1]\n",
)
assert repository.is_ahead_or_behind() is True
def test_is_synced_when_synced(fp, repository):
fp.register(
["git", "status", "--branch", "--porcelain"], stdout="## main...origin/main\n"
)
fp.register(
["git", "status", "--branch", "--porcelain"], stdout="## main...origin/main\n"
)
assert repository.is_synced() is True
def test_is_synced_with_uncommitted_changes(fp, repository):
fp.register(
["git", "status", "--branch", "--porcelain"],
stdout="## main...origin/main\n M somefile.txt\n",
)
assert repository.is_synced() is False
def test_is_synced_when_ahead_or_behind(fp, repository):
fp.register(
["git", "status", "--branch", "--porcelain"],
stdout="## main...origin/main [ahead 1]\n",
)
fp.register(
["git", "status", "--branch", "--porcelain"],
stdout="## main...origin/main [ahead 1]\n",
)
assert repository.is_synced() is False
def test_is_synced_with_uncommitted_changes_and_ahead(fp, repository):
fp.register(
["git", "status", "--branch", "--porcelain"],
stdout="## main...origin/main [ahead 1]\n M somefile.txt\n",
)
assert repository.is_synced() is False
def test_origin_url(fp, repository):
fp.register(
["git", "remote", "get-url", "origin"],
stdout="https://github.com/user/repo.git\n",
)
assert repository.origin_url() == "https://github.com/user/repo.git"

View File

@@ -129,8 +129,10 @@ class TestToolCommand(unittest.TestCase):
read_data=b"sample tarball content", read_data=b"sample tarball content",
) )
@patch("crewai.cli.plus_api.PlusAPI.publish_tool") @patch("crewai.cli.plus_api.PlusAPI.publish_tool")
@patch("crewai.cli.tools.main.git.Repository.is_synced", return_value=True)
def test_publish_success( def test_publish_success(
self, self,
mock_is_synced,
mock_publish, mock_publish,
mock_open, mock_open,
mock_listdir, mock_listdir,
@@ -147,16 +149,16 @@ class TestToolCommand(unittest.TestCase):
tool_command = ToolCommand() tool_command = ToolCommand()
tool_command.publish(is_public=True) tool_command.publish(is_public=True)
mock_get_project_name.assert_called_once_with(require=True) mock_get_project_name.assert_called_with(require=True)
mock_get_project_version.assert_called_once_with(require=True) mock_get_project_version.assert_called_with(require=True)
mock_get_project_description.assert_called_once_with(require=False) mock_get_project_description.assert_called_with(require=False)
mock_subprocess_run.assert_called_once_with( mock_subprocess_run.assert_called_with(
["poetry", "build", "-f", "sdist", "--output", unittest.mock.ANY], ["poetry", "build", "-f", "sdist", "--output", unittest.mock.ANY],
check=True, check=True,
capture_output=False, capture_output=False,
) )
mock_open.assert_called_once_with(unittest.mock.ANY, "rb") mock_open.assert_called_with(unittest.mock.ANY, "rb")
mock_publish.assert_called_once_with( mock_publish.assert_called_with(
handle="sample-tool", handle="sample-tool",
is_public=True, is_public=True,
version="1.0.0", version="1.0.0",