Skip to content

Send in the correct and expected types #499

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Parent: Merge develop into infrahub-develop
merged 1 commit into from
Aug 15, 2025
Merged

Send in the correct and expected types #499

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions infrahub_sdk/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def decode_json(response: httpx.Response) -> dict:
try:
return response.json()
except json.decoder.JSONDecodeError as exc:
raise JsonDecodeError(content=response.text, url=response.url) from exc
raise JsonDecodeError(content=response.text, url=str(response.url)) from exc


def generate_uuid() -> str:
Expand Down Expand Up @@ -232,7 +232,7 @@ def get_branch(branch: str | None = None, directory: str | Path = ".") -> str:
if branch:
return branch

repo = GitRepoManager(directory)
repo = GitRepoManager(root_directory=str(directory))
return str(repo.active_branch)
Comment on lines +235 to 236
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Bug: returning "None" string when no active branch is present

repo.active_branch is annotated as str | None (see infrahub_sdk/repository.py). Casting it with str(...) will convert None to the literal string "None", which is misleading and violates the function’s contract to return the actual branch name. Handle the None case explicitly.

Apply this diff to avoid returning "None" and fail fast with a clear error:

-    repo = GitRepoManager(root_directory=str(directory))
-    return str(repo.active_branch)
+    repo = GitRepoManager(root_directory=str(directory))
+    active_branch = repo.active_branch
+    if active_branch is None:
+        raise ValueError(f"Could not determine active branch for repository at {directory!s}")
+    return active_branch

Run this to find other call sites that may still use the old GitRepoManager constructor or rely on get_branch returning a non-empty string:

🏁 Script executed:

#!/bin/bash
# Find GitRepoManager instantiations and check argument usage
rg -n -C2 '\bGitRepoManager\s*\('

# Look for any usage expecting get_branch() to never be empty/None
rg -n -C2 '\bget_branch\s*\('

Length of output: 5976

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- infrahub_sdk/utils.py (1-400) ----"
sed -n '1,400p' infrahub_sdk/utils.py || true

echo
echo "---- infrahub_sdk/repository.py (1-240) ----"
sed -n '1,240p' infrahub_sdk/repository.py || true

echo
echo "---- infrahub_sdk/operation.py (1-160) ----"
sed -n '1,160p' infrahub_sdk/operation.py || true

echo
echo "---- infrahub_sdk/checks.py (1-240) ----"
sed -n '1,240p' infrahub_sdk/checks.py || true

echo
echo "---- infrahub_sdk/node/node.py (1-140) ----"
sed -n '1,140p' infrahub_sdk/node/node.py || true

echo
echo "---- infrahub_sdk/store.py (1-320) ----"
sed -n '1,320p' infrahub_sdk/store.py || true

echo
echo "---- Grep for active_branch usages ----"
rg -n --hidden -S 'active_branch' || true

echo
echo "---- Grep for str(...active_branch...) usages ----"
rg -n --hidden -S "str\([^)]+active_branch[^\)]*\)" || true

Length of output: 39725

Fail fast on missing git active branch — avoid returning the literal "None"

Casting repo.active_branch with str(...) will convert None to the string "None". repository.active_branch is annotated as str | None (infrahub_sdk/repository.py), and I found three call sites that currently do this conversion — they must explicitly handle None and fail fast with a clear error.

Files to change:

  • infrahub_sdk/utils.py — get_branch (around lines 230–236)
  • infrahub_sdk/operation.py — branch_name property (around line 42)
  • infrahub_sdk/checks.py — branch_name property (around line 143)

Suggested diffs:

infrahub_sdk/utils.py

-    repo = GitRepoManager(root_directory=str(directory))
-    return str(repo.active_branch)
+    repo = GitRepoManager(root_directory=str(directory))
+    active_branch = repo.active_branch
+    if active_branch is None:
+        raise ValueError(f"Could not determine active branch for repository at {directory!s}")
+    return active_branch

infrahub_sdk/operation.py

-        if not hasattr(self, "git") or not self.git:
-            self.git = GitRepoManager(self.root_directory)
-
-        self.branch = str(self.git.active_branch)
+        if not hasattr(self, "git") or not self.git:
+            self.git = GitRepoManager(self.root_directory)
+
+        active_branch = self.git.active_branch
+        if active_branch is None:
+            raise ValueError(f"Could not determine active branch for repository at {self.root_directory!s}")
+        self.branch = active_branch

infrahub_sdk/checks.py

-        if not self.git:
-            self.git = GitRepoManager(self.root_directory)
-
-        self.branch = str(self.git.active_branch)
-        return self.branch
+        if not self.git:
+            self.git = GitRepoManager(self.root_directory)
+
+        active_branch = self.git.active_branch
+        if active_branch is None:
+            raise ValueError(f"Could not determine active branch for repository at {self.root_directory!s}")
+        self.branch = active_branch
+        return self.branch

Reasoning: explicit None checks avoid returning the misleading string "None" and produce a clear, early error when the active branch cannot be determined. After these changes, update any callers that expect get_branch()/branch_name to always succeed or handle the ValueError appropriately.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
repo = GitRepoManager(root_directory=str(directory))
return str(repo.active_branch)
repo = GitRepoManager(root_directory=str(directory))
active_branch = repo.active_branch
if active_branch is None:
raise ValueError(f"Could not determine active branch for repository at {directory!s}")
return active_branch
🤖 Prompt for AI Agents 
infrahub_sdk/utils.py around lines 230–236, infrahub_sdk/operation.py around
line 42, and infrahub_sdk/checks.py around line 143: currently code casts
repository.active_branch to str which turns None into the literal "None";
instead, check if repo.active_branch is None and if so raise a clear ValueError
(e.g. "cannot determine active git branch") so callers fail fast, otherwise
return the branch string; update the three locations to perform this explicit
None check and raise the ValueError rather than returning str(None).



Expand Down