Refactored BugZooka inference with py-commons#115
Conversation
|
Warning Review limit reached
More reviews will be available in 28 minutes and 46 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR migrates the inference client implementation and configuration management from BugZooka into a shared py-commons library. The core inference client, exception types, and base configuration are now re-exported from commons.inference, while BugZooka retains only its custom retry policy layer. Async execution switches from asyncio to anyio, and environment variable naming changes from ChangesInference commons migration and async execution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors BugZooka’s inference integration to delegate the inference client implementation to the shared py-commons inference module, while keeping BugZooka’s public imports/backwards compatibility intact. It also updates sync↔async bridging code paths to use anyio.run(...) and renames the inference timeout environment variable.
Changes:
- Replaced BugZooka’s in-repo inference client implementation with re-exports from
py-commons(commons.inference). - Renamed
INFERENCE_API_TIMEOUT_SECONDS→INFERENCE_API_TIMEOUTacross docs/config and aligned retry config sourcing. - Switched several “run async from sync” call sites to
anyio.run(...)(and introduced new dependency expectations).
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| requirements.txt | Adds py-commons and cryptography dependencies. |
| README.md | Updates env var name and updates repo tree description of inference client. |
| CONTRIBUTING.md | Updates env var name for timeout. |
| bugzooka/integrations/slack_socket_listener.py | Uses anyio.run(...) instead of manually creating event loops (also introduces an unused import). |
| bugzooka/integrations/inference_client.py | Replaces prior client implementation with py-commons re-exports + compatibility aliases. |
| bugzooka/core/constants.py | Removes local inference constants (now sourced from py-commons) and keeps retry constants. |
| bugzooka/core/config.py | Delegates base inference config parsing to py-commons, then overlays BugZooka retry settings. |
| bugzooka/analysis/log_analyzer.py | Retry config now comes from inference config; switches asyncio.run → anyio.run (also introduces an unused import). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import asyncio | ||
| import concurrent.futures | ||
| import logging |
| import logging | ||
| import asyncio | ||
| from pydantic import BaseModel, Field |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@requirements.txt`:
- Around line 18-20: requirements.txt lists an unsatisfiable py-commons
constraint and an unclear cryptography bound; replace "py-commons>=0.3.0" with a
valid PyPI release (e.g., "py-commons==0.1.2" or "py-commons>=0.1.2,<0.2.0") and
update the cryptography line to include concrete advisory-backed bounds (e.g.,
change "cryptography>=44.0.0" to a supported safe range like
"cryptography>=X.Y.Z,<A.B.C" where X.Y.Z is the minimum secure version and A.B.C
the next breaking version) and add a brief comment referencing the exact
CVE/advisory URL that justifies the bound so CI/installers can resolve and
auditors can verify the rationale.
- Around line 46-47: Update the cryptography minimum version in requirements.txt
by replacing the current declaration "cryptography>=44.0.0" with a safe floor
such as "cryptography>=46.0.6" (or alternatively specify a range that excludes
vulnerable releases, e.g., "cryptography>=46.0.6,<47.0") so the project will not
install known-vulnerable 44.x/46.x releases; locate and edit the line containing
the literal package spec "cryptography>=44.0.0" to apply this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: f572068f-0766-4164-8485-e7a5602395ab
📒 Files selected for processing (8)
CONTRIBUTING.mdREADME.mdbugzooka/analysis/log_analyzer.pybugzooka/core/config.pybugzooka/core/constants.pybugzooka/integrations/inference_client.pybugzooka/integrations/slack_socket_listener.pyrequirements.txt
| # py-commons | ||
| py-commons>=0.3.0 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether py-commons 0.3.0 is published on PyPI.
status="$(curl -s -o /tmp/pycommons.json -w "%{http_code}" https://pypi.org/pypi/py-commons/json || true)"
echo "PyPI status code for py-commons metadata: ${status}"
if [ "$status" = "200" ]; then
jq -r '.releases | keys[]' /tmp/pycommons.json | tail -n 20
echo "Check whether 0.3.0 appears in the list above."
else
echo "Package metadata unavailable on PyPI (status ${status})."
fiRepository: redhat-performance/BugZooka
Length of output: 176
Fix broken dependency constraint: py-commons>=0.3.0
requirements.txtline 19 usespy-commons>=0.3.0, but PyPI only shows releases0.1.0and0.1.2, so installs/CI using PyPI won’t be able to satisfy this constraint.requirements.txtline 47 usescryptography>=44.0.0; the vulnerability/exclusion rationale for44.0.0needs a concrete advisory/source and corresponding bounds update.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@requirements.txt` around lines 18 - 20, requirements.txt lists an
unsatisfiable py-commons constraint and an unclear cryptography bound; replace
"py-commons>=0.3.0" with a valid PyPI release (e.g., "py-commons==0.1.2" or
"py-commons>=0.1.2,<0.2.0") and update the cryptography line to include concrete
advisory-backed bounds (e.g., change "cryptography>=44.0.0" to a supported safe
range like "cryptography>=X.Y.Z,<A.B.C" where X.Y.Z is the minimum secure
version and A.B.C the next breaking version) and add a brief comment referencing
the exact CVE/advisory URL that justifies the bound so CI/installers can resolve
and auditors can verify the rationale.
Source: Coding guidelines
| # Misc. | ||
| cryptography>=44.0.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Query OSV for vulnerabilities affecting cryptography 44.0.0
curl -s https://api.osv.dev/v1/query \
-H 'Content-Type: application/json' \
-d '{"package":{"name":"cryptography","ecosystem":"PyPI"},"version":"44.0.0"}' \
| jq '{affected_version:"44.0.0", vulns:[.vulns[]?.id]}'Repository: redhat-performance/BugZooka
Length of output: 225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm requirements.txt constraint around the cited lines
echo "---- requirements.txt (lines 35-60) ----"
nl -ba requirements.txt | sed -n '35,60p'
# 2) Check for any other cryptography constraints in the repo
echo "---- grep for cryptography constraints ----"
rg -n --hidden --no-ignore-vcs -S "cryptography\s*(?:[<>=!~]=?|\[\s*)\s*" .
# 3) Query OSV for cryptography 44.0.0 with full vuln payload (not just IDs)
echo "---- OSV response for cryptography 44.0.0 (full) ----"
curl -s https://api.osv.dev/v1/query \
-H 'Content-Type: application/json' \
-d '{"package":{"name":"cryptography","ecosystem":"PyPI"},"version":"44.0.0"}' \
| jq '.vulns'
# 4) Try likely nearby patch versions to identify a “safe” floor (best-effort)
for v in 44.0.1 44.0.2 44.0.3 44.1.0 44.1.1; do
echo "---- OSV vulns for cryptography $v ----"
curl -s https://api.osv.dev/v1/query \
-H 'Content-Type: application/json' \
-d "{\"package\":{\"name\":\"cryptography\",\"ecosystem\":\"PyPI\"},\"version\":\"$v\"}" \
| jq -r --arg v "$v" '{version:$v, count:(.vulns|length), vulns:(.vulns[].id // empty)}'
doneRepository: redhat-performance/BugZooka
Length of output: 173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- requirements.txt (lines 35-60) ----"
cat -n requirements.txt | sed -n '35,60p'
echo "---- grep for cryptography constraints ----"
rg -n --hidden --no-ignore-vcs -S "cryptography\s*(?:[<>=!~]=?|\[\s*)\s*" .
echo "---- OSV response for cryptography 44.0.0 (full) ----"
curl -s https://api.osv.dev/v1/query \
-H 'Content-Type: application/json' \
-d '{"package":{"name":"cryptography","ecosystem":"PyPI"},"version":"44.0.0"}' \
| jq '{vulns:.vulns, affected_version:"44.0.0"}'
echo "---- OSV extract fixed/patched hints for vulns affecting 44.0.0 ----"
curl -s https://api.osv.dev/v1/query \
-H 'Content-Type: application/json' \
-d '{"package":{"name":"cryptography","ecosystem":"PyPI"},"version":"44.0.0"}' \
| jq -r '
.vulns[]
| {id, summary, details: .details?} as $v
| $v,
(.affected[]? // empty) as $a
| (if ($a.ranges) then
$a.ranges[]
| {range_type:(.type // null), repo_type:(.repo?.type // null), events:(.events // null)}
else empty end)
' | head -n 200Repository: redhat-performance/BugZooka
Length of output: 27007
Update cryptography minimum to a patched version (security fix)
requirements.txt line 47 (cryptography>=44.0.0) allows installing 44.0.0, which OSV lists as affected by multiple advisories, including:
- GHSA-79v4-65xg-pq4g (fixed in
44.0.1) - GHSA-r6ph-v2qm-q3c2 (fixed in
46.0.5, HIGH) - GHSA-m959-cc7f-wv43 / PYSEC-2026-35 (fixed in
46.0.6)
Set the floor to at least cryptography>=46.0.6 (or explicitly exclude the affected versions).
🧰 Tools
🪛 OSV Scanner (2.3.8)
[HIGH] 47-47: cryptography 44.0.0: undefined
(PYSEC-2026-35)
[HIGH] 47-47: cryptography 44.0.0: Vulnerable OpenSSL included in cryptography wheels
[HIGH] 47-47: cryptography 44.0.0: cryptography has incomplete DNS name constraint enforcement on peer names
[HIGH] 47-47: cryptography 44.0.0: cryptography Vulnerable to a Subgroup Attack Due to Missing Subgroup Validation for SECT Curves
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@requirements.txt` around lines 46 - 47, Update the cryptography minimum
version in requirements.txt by replacing the current declaration
"cryptography>=44.0.0" with a safe floor such as "cryptography>=46.0.6" (or
alternatively specify a range that excludes vulnerable releases, e.g.,
"cryptography>=46.0.6,<47.0") so the project will not install known-vulnerable
44.x/46.x releases; locate and edit the line containing the literal package spec
"cryptography>=44.0.0" to apply this change.
Sources: Coding guidelines, Linters/SAST tools
| ) | ||
| finally: | ||
| loop.close() | ||
| analysis_result = anyio.run( |
There was a problem hiding this comment.
Why are we changing this? @qu4rkn3t do you see any benefits out of this w.r.t previous function?
There was a problem hiding this comment.
When testing analyze_pr, I was getting the following error:
TaskGroup is an anyio async context manager, so I thought if I used that it would help. Turns out it did. I suspect it's because mcp and/or langchain-mcp-adapters use anyio under the hood and using asyncio created some duality. I suppose the implementation is also cleaner without needing to new/set/close.
There was a problem hiding this comment.
Indeed. lets see how it goes over time.
| ) | ||
| finally: | ||
| loop.close() | ||
| analysis_result = anyio.run( |
There was a problem hiding this comment.
Indeed. lets see how it goes over time.
vishnuchalla
left a comment
vishnuchalla
left a comment
There was a problem hiding this comment.
@qu4rkn3t mind fixing the lint issues real quick please?
I'm not sure how to as it's caused by latest py-commons not existing in PyPi. I re-ran the publishing job, but I get the following Node.js error: |
Can you try now? updated py-commons release that should have the fix. |
No, still running into the same issue. |
|
I re-ran v.0.1.8 and I now get this: |
Description
Updated BugZooka to use py-commons inference module
inference_client.py(removed old client and exposedpy-commonsAPIs)INFERENCE_API_TIMEOUT_SECONDStoINFERENCE_API_TIMEOUTto match other time variables. Unit is explicitly stated in py-commons and the variable is updated where needed in BugZooka.make install. To circumvent, clone py-commons and use pip to manually install the local package.Example
Below is an
analyze_prcall, which works exactly like the dev BugZooka client.