Skip to content

Conversation

@ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Aug 28, 2025

Fixes #236.

Summary by CodeRabbit

  • Bug Fixes

    • Ensured all GraphQL requests (async and sync) always include a branch parameter, defaulting to the configured default branch when none is provided. This delivers consistent branch-scoped behavior across requests and prevents unintended cross-branch queries; other request parameters remain unchanged.
  • Documentation

    • Added changelog entry noting respect for the default branch in relevant client operations.

@coderabbitai
Copy link

coderabbitai bot commented Aug 28, 2025

Walkthrough

The change updates both async and sync GraphQL query builders in infrahub_sdk/client.py to always include a branch URL parameter, setting it to the provided branch_name or falling back to self.default_branch. set_context_properties likewise passes branch=branch or self.default_branch. Other URL parameters (at, subscribers, update_group) and public signatures remain unchanged.

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7ae0053 and 4dde424.

📒 Files selected for processing (2)
  • changelog/236.fixed.md (1 hunks)
  • infrahub_sdk/client.py (3 hunks)
✅ Files skipped from review due to trivial changes (1)
  • changelog/236.fixed.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • infrahub_sdk/client.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: unit-tests (3.13)
  • GitHub Check: unit-tests (3.11)
  • GitHub Check: unit-tests (3.10)
  • GitHub Check: unit-tests (3.12)
  • GitHub Check: integration-tests-latest-infrahub
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pog-query-branch-IHS-89

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbit summary or Summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 28, 2025

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4dde424
Status: ✅  Deploy successful!
Preview URL: https://80159827.infrahub-sdk-python.pages.dev
Branch Preview URL: https://pog-query-branch-ihs-89.infrahub-sdk-python.pages.dev

View logs

@codecov
Copy link

codecov bot commented Aug 28, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/client.py 50.00% 1 Missing ⚠️
@@            Coverage Diff             @@
##           stable     #518      +/-   ##
==========================================
+ Coverage   75.76%   75.77%   +0.01%     
==========================================
  Files         100      100              
  Lines        8858     8856       -2     
  Branches     1737     1735       -2     
==========================================
  Hits         6711     6711              
+ Misses       1670     1669       -1     
+ Partials      477      476       -1     
Flag Coverage Δ
integration-tests 34.75% <0.00%> (+<0.01%) ⬆️
python-3.10 48.28% <50.00%> (+0.01%) ⬆️
python-3.11 48.28% <50.00%> (+0.01%) ⬆️
python-3.12 48.23% <50.00%> (+0.01%) ⬆️
python-3.13 48.23% <50.00%> (+0.01%) ⬆️
python-3.9 46.95% <50.00%> (+0.03%) ⬆️
python-filler-3.12 25.06% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/client.py 69.64% <50.00%> (+0.15%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 482c969 and 7ae0053.

📒 Files selected for processing (1)
  • infrahub_sdk/client.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

When implementing Infrahub checks, subclass InfrahubCheck and override validate(data); do not implement or rely on a check() method

Files:

  • infrahub_sdk/client.py
infrahub_sdk/client.py

📄 CodeRabbit inference engine (CLAUDE.md)

infrahub_sdk/client.py: Use HTTPX for transport with proxy support (single proxy or HTTP/HTTPS mounts)
Support authentication via API tokens or JWT with automatic refresh

Files:

  • infrahub_sdk/client.py
🧬 Code graph analysis (1)
infrahub_sdk/client.py (2)
infrahub_sdk/operation.py (1)
  • branch_name (33-44)
infrahub_sdk/checks.py (1)
  • branch_name (134-144)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: unit-tests (3.10)
  • GitHub Check: unit-tests (3.12)
  • GitHub Check: unit-tests (3.13)
  • GitHub Check: integration-tests-latest-infrahub
🔇 Additional comments (1)
infrahub_sdk/client.py (1)

1106-1107: Don’t clobber caller-supplied params['branch']; only default when missing.

Apply:

-        url_params["branch"] = branch_name or self.default_branch
+        if branch_name:
+            url_params["branch"] = branch_name
+        elif "branch" not in url_params and self.default_branch:
+            url_params["branch"] = self.default_branch

Verify no callers rely on passing branch via params.

Comment on lines +2245 to +2246
url_params["branch"] = branch_name or self.default_branch

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

Mirror the non-breaking precedence in sync path.

Same concern and fix as async; keep params['branch'] if provided, else fall back to default.

-        url_params["branch"] = branch_name or self.default_branch
+        if branch_name:
+            url_params["branch"] = branch_name
+        elif "branch" not in url_params and self.default_branch:
+            url_params["branch"] = self.default_branch

Optional follow-up: extract this small precedence block into a BaseClient helper to DRY both call sites.


🏁 Script executed:

#!/bin/bash
# Locate client.py file
fd -g client.py infrahub_sdk

# Search for sync branch assignment
rg -n 'url_params\["branch"\]' -C5 infrahub_sdk/client.py

# Search for branch_name usage
rg -n 'branch_name' -C5 infrahub_sdk/client.py

# Search for async functions where branch is set
rg -n 'async def' -C5 infrahub_sdk/client.py

Length of output: 35479


Apply conditional branch precedence in both REST query methods

– In infrahub_sdk/client.py around line 1106 (async query_gql_query) and line 2245 (sync REST query), replace

url_params["branch"] = branch_name or self.default_branch

with:

if branch_name:
    url_params["branch"] = branch_name
elif "branch" not in url_params and self.default_branch:
    url_params["branch"] = self.default_branch

– Optional: extract this block into a BaseClient helper to DRY both call sites.

🤖 Prompt for AI Agents
In infrahub_sdk/client.py around lines 1106 (async query_gql_query) and 2245
(sync REST query), the branch parameter is unconditionally set using
"branch_name or self.default_branch"; change it to conditionally prefer an
explicit branch_name, only add default_branch if no "branch" already exists in
url_params and self.default_branch is set (i.e., if branch_name: set
url_params["branch"]=branch_name; elif "branch" not in url_params and
self.default_branch: set url_params["branch"]=self.default_branch). Apply the
same logic to both call sites, and optionally extract this logic into a
BaseClient helper method to avoid duplication and call it from both locations.

@ogenstad ogenstad force-pushed the pog-query-branch-IHS-89 branch from 7ae0053 to 4dde424 Compare August 28, 2025 11:53
@ogenstad ogenstad requested a review from a team August 28, 2025 12:17
@ajtmccarty
Copy link
Contributor

it looks like at least some of the client-related code expects the default_branch property of the client to actually be the default branch name (main in most cases) and not "the branch to use for this generator/transform/whatever"

for example
in BaseClient.__init__

        self.default_branch = self.config.default_infrahub_branch

in InfrahubClient._initialize

    self.store = NodeStore(default_branch=self.default_branch)

where NodeStore seems to expect the default_branch parameter to actually be the name of the default branch

so maybe what we really need is a client config option like, execution_branch or something to cover this use case

@ogenstad ogenstad merged commit f4035f0 into stable Sep 4, 2025
21 checks passed
@ogenstad ogenstad deleted the pog-query-branch-IHS-89 branch September 4, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Function query_gql_query doesn't take into account client config

2 participants