Skip to content

Commit

Permalink
make lint now checks for format (#656)
Browse files Browse the repository at this point in the history
This enables `make lint` to also run black but without persisting the
changes. This will hopefully allow developers to more easily detect if
their changes are not compliant with the linter settings.

This also removes the `make format` call from CI, which didn't actually
fail if linter issues were found.

Signed-off-by: Juan Antonio Osorio <[email protected]>
  • Loading branch information
JAORMX authored Jan 20, 2025
1 parent 9fab905 commit 54e389e
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 21 deletions.
3 changes: 0 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ jobs:
- name: Run linting
run: make lint

- name: Run formatting
run: make format

- name: Run tests
run: make test

Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ format:
poetry run ruff check --fix .

lint:
poetry run black --check .
poetry run ruff check .

test:
Expand Down
9 changes: 6 additions & 3 deletions src/codegate/api/v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,12 @@ async def create_workspace(request: v1_models.CreateWorkspaceRequest) -> v1_mode
except AlreadyExistsError:
raise HTTPException(status_code=409, detail="Workspace already exists")
except ValidationError:
raise HTTPException(status_code=400,
detail=("Invalid workspace name. "
"Please use only alphanumeric characters and dashes"))
raise HTTPException(
status_code=400,
detail=(
"Invalid workspace name. " "Please use only alphanumeric characters and dashes"
),
)
except Exception:
raise HTTPException(status_code=500, detail="Internal server error")

Expand Down
15 changes: 11 additions & 4 deletions src/codegate/db/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@
alert_queue = asyncio.Queue()
fim_cache = FimCache()


class AlreadyExistsError(Exception):
pass


class DbCodeGate:
_instance = None

Expand Down Expand Up @@ -266,7 +268,8 @@ async def add_workspace(self, workspace_name: str) -> Optional[Workspace]:

try:
added_workspace = await self._execute_update_pydantic_model(
workspace, sql, should_raise=True)
workspace, sql, should_raise=True
)
except IntegrityError as e:
logger.debug(f"Exception type: {type(e)}")
raise AlreadyExistsError(f"Workspace {workspace_name} already exists.")
Expand Down Expand Up @@ -317,8 +320,11 @@ async def _execute_select_pydantic_model(
return None

async def _exec_select_conditions_to_pydantic(
self, model_type: Type[BaseModel], sql_command: TextClause, conditions: dict,
should_raise: bool = False
self,
model_type: Type[BaseModel],
sql_command: TextClause,
conditions: dict,
should_raise: bool = False,
) -> Optional[List[BaseModel]]:
async with self._async_db_engine.begin() as conn:
try:
Expand Down Expand Up @@ -397,7 +403,8 @@ async def get_workspace_by_name(self, name: str) -> List[Workspace]:
)
conditions = {"name": name}
workspaces = await self._exec_select_conditions_to_pydantic(
Workspace, sql, conditions, should_raise=True)
Workspace, sql, conditions, should_raise=True
)
return workspaces[0] if workspaces else None

async def get_sessions(self) -> List[Session]:
Expand Down
2 changes: 1 addition & 1 deletion src/codegate/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,4 @@ def generate_openapi():

# Convert the schema to JSON string for easier handling or storage
openapi_json = json.dumps(openapi_schema, indent=2)
print(openapi_json)
print(openapi_json)
8 changes: 5 additions & 3 deletions src/codegate/workspaces/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@
class WorkspaceCrudError(Exception):
pass


class WorkspaceDoesNotExistError(WorkspaceCrudError):
pass


class WorkspaceAlreadyActiveError(WorkspaceCrudError):
pass


class WorkspaceCrud:

def __init__(self):
Expand All @@ -30,7 +33,7 @@ async def add_workspace(self, new_workspace_name: str) -> Workspace:
workspace_created = await db_recorder.add_workspace(new_workspace_name)
return workspace_created

async def get_workspaces(self)-> List[WorkspaceActive]:
async def get_workspaces(self) -> List[WorkspaceActive]:
"""
Get all workspaces
"""
Expand Down Expand Up @@ -60,8 +63,7 @@ async def _is_workspace_active(
raise RuntimeError("Something went wrong. No active session found.")

session = sessions[0]
return (session.active_workspace_id == selected_workspace.id,
session, selected_workspace)
return (session.active_workspace_id == selected_workspace.id, session, selected_workspace)

async def activate_workspace(self, workspace_name: str):
"""
Expand Down
8 changes: 2 additions & 6 deletions tests/pipeline/workspace/test_workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ async def test_add_workspaces(args, existing_workspaces, expected_message):
workspace_commands._db_reader = mock_db_reader

# We'll also patch DbRecorder to ensure no real DB operations happen
with patch(
"codegate.workspaces.crud.WorkspaceCrud", autospec=True
) as mock_recorder_cls:
with patch("codegate.workspaces.crud.WorkspaceCrud", autospec=True) as mock_recorder_cls:
mock_recorder = mock_recorder_cls.return_value
workspace_commands.workspace_crud = mock_recorder
mock_recorder.add_workspace = AsyncMock()
Expand Down Expand Up @@ -115,9 +113,7 @@ async def test_parse_execute_cmd(
"""
workspace_commands = Workspace()

with patch.object(
workspace_commands, "run", return_value=mocked_execute_response
) as mock_run:
with patch.object(workspace_commands, "run", return_value=mocked_execute_response) as mock_run:
result = await workspace_commands.exec(user_message)
assert result == mocked_execute_response

Expand Down
4 changes: 3 additions & 1 deletion tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def test_health_check(test_client: TestClient) -> None:
assert response.status_code == 200
assert response.json() == {"status": "healthy"}


@patch("codegate.api.dashboard.dashboard.fetch_latest_version", return_value="foo")
def test_version_endpoint(mock_fetch_latest_version, test_client: TestClient) -> None:
"""Test the version endpoint."""
Expand All @@ -89,11 +90,12 @@ def test_version_endpoint(mock_fetch_latest_version, test_client: TestClient) ->

response_data = response.json()

assert response_data["current_version"] == __version__.lstrip('v')
assert response_data["current_version"] == __version__.lstrip("v")
assert response_data["latest_version"] == "foo"
assert isinstance(response_data["is_latest"], bool)
assert response_data["is_latest"] is False


@patch("codegate.pipeline.secrets.manager.SecretsManager")
@patch("codegate.server.ProviderRegistry")
def test_provider_registration(mock_registry, mock_secrets_mgr, mock_pipeline_factory) -> None:
Expand Down

0 comments on commit 54e389e

Please sign in to comment.