v1.5.2: fix missing exceptiongroup dependency for Py3.10 #163
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughBump project version to 1.5.2, add runtime dependency Changes
Sequence Diagram(s)sequenceDiagram
participant Test as TestCase.setUp
participant MinIO as MinIOClient
Note over Test,MinIO `#DDEEFF`: Test setup ensures bucket presence
Test->>MinIO: bucket_exists(bucket)
alt bucket exists
MinIO-->>Test: True
else bucket missing
MinIO-->>Test: False
Test->>MinIO: create_bucket(bucket)
MinIO-->>Test: created
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
python/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
python/pyproject.toml(3 hunks)
⏰ 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: build (macos-latest, 3.11)
- GitHub Check: build (macos-latest, 3.10)
- GitHub Check: build (windows-latest, 3.11)
- GitHub Check: build (windows-latest, 3.14)
🔇 Additional comments (3)
python/pyproject.toml (3)
3-3: Version bump to 1.5.2 looks straightforward.Clean version increment aligned with the exceptiongroup fix. The comment about sync with commitizen config at line 81 suggests this is auto-managed—good. Just ensure your CI actually enforces this sync.
19-19: The exceptiongroup dependency addition is correct and actively used—no issues found.The verification confirms
exceptiongroupis properly imported and used throughout the codebase:
- Used in production code:
python/bucketbase/backup_multi_bucket.pyraisesExceptionGroupin exception handlers with the correct conditional import pattern- Used in tests:
python/tests/test_backup_multi_bucket.pyvalidates exception behavior- Conditional import pattern is correct: The code uses
_ = ExceptionGroup.__class__before importing fromexceptiongroup, which properly handles Python 3.11+ (built-in) vs. <3.11 (backport) compatibilityThe version constraint
>=1.0.0is reasonable for stable releases. While you could tighten it to>=1.0.0,<2.0.0if desired, there's no evidence this is necessary. The current constraint aligns with standard practices.
28-28: Clarify tsx package source before verification.Given that
tsxis in a Python project'spyproject.toml, this is likely the esbuild-kit npm package used via a Node dependency manager. Let me search for the specific changelog:Let me search more specifically for those older tsx versions:
The search results show that <13-1>"tsx · 4.20.5 (latest) 4.20.4 · 4.20.3 · 4.20.2 · 4.20.1 · 4.20.0 · 4.19.4 · 4.19.3 · 4.19.2 · 4.19.1 · 4.19.0 · 4.18.0"</13-1> — versions starting from 3.11.0+. Versions 0.2.12 and 0.2.14 don't appear in the tsx npm package's version history. Let me verify if this might be a different package entirely:
Review comment is incorrect—tsx versions 0.2.12 and 0.2.14 do not exist.
The investigation reveals that <22-3,29-7>tsx npm package version history begins at 1.0.0 and goes forward, with no 0.x versions</22-3,29-7>. <21-2>The latest version is 4.20.6</21-2>. The versions 0.2.12 and 0.2.14 referenced in
pyproject.tomlcannot be verified because they don't exist in the tsx npm package registry.Either the version numbers in the file are incorrect, or this references a different package entirely. The original review comment's request to check for breaking changes between non-existent versions is invalid.
Likely an incorrect or invalid review comment.
| minio = "^7.2.18" | ||
| multiminio = "^1.0.2" | ||
| tsx = ">=0.2.12" | ||
| tsx = ">=0.2.14" |
There was a problem hiding this comment.
🧩 Analysis chain
🚨 CRITICAL: tsx version numbers (0.2.12 → 0.2.14) don't appear to exist.
The latest version of tsx on npm is 4.20.6, so >=0.2.12 and >=0.2.14 don't exist in the package registry. This is either:
- A typo (maybe you meant
4.0.12or4.0.14?) - An internal/forked version of tsx that's not published to npm
- A different package entirely
This will fail at dependency resolution time and break builds. Clarify the actual intended version before merging.
Verify the actual tsx package version by checking the package registry:
🏁 Script executed:
#!/bin/bash
# Fetch tsx package info from npm
curl -s https://registry.npmjs.org/tsx | jq '.versions | keys[-10:]'Length of output: 112
🚨 CRITICAL: tsx version constraint >=0.2.14 references non-existent versions.
The tsx package on npm only has versions in the 4.x range (latest verified: 4.9.5). Versions 0.2.12 and 0.2.14 do not exist in the registry. This dependency constraint will fail at resolution time and break the build.
Either:
- Update to an actual tsx version (e.g.,
>=4.9.0) - Verify this should be a different package entirely
This needs to be corrected before merging—dependency resolution will fail otherwise.
🤖 Prompt for AI Agents
In python/pyproject.toml around line 28, the tsx version constraint ">=0.2.14"
references non-existent 0.x releases and will break dependency resolution;
replace it with a valid tsx semver (for example ">=4.9.0" or a specific 4.x
version) or, if "tsx" was a mistaken package name, change it to the correct
package; update the constraint to a real published version, run dependency
resolution to verify it installs, and commit the corrected pyproject.toml.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
python/tests/test_minio_bucket.py(1 hunks)
⏰ 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: build (windows-latest, 3.13)
- GitHub Check: build (macos-latest, 3.11)
- GitHub Check: build (macos-latest, 3.10)
- GitHub Check: build (windows-latest, 3.11)
- GitHub Check: build (windows-latest, 3.12)
| if not self.minio_client.bucket_exists(CONFIG.MINIO_DEV_TESTS_BUCKET): | ||
| self.minio_client.make_bucket(bucket_name=CONFIG.MINIO_DEV_TESTS_BUCKET) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Add error handling and consider race conditions.
Look, this defensive bucket creation is smart—no one likes flaky tests—but there's a classic TOCTOU issue here. If you're running tests in parallel (and you should be, time is money), two processes could both see the bucket doesn't exist and both try to create it. One will fail.
Also, zero error handling around make_bucket? That's like launching a rocket without checking if the launchpad is clear. Any bucket creation error will just blow up the test setup with no context.
Consider this more robust approach:
- if not self.minio_client.bucket_exists(CONFIG.MINIO_DEV_TESTS_BUCKET):
- self.minio_client.make_bucket(bucket_name=CONFIG.MINIO_DEV_TESTS_BUCKET)
+ try:
+ if not self.minio_client.bucket_exists(CONFIG.MINIO_DEV_TESTS_BUCKET):
+ self.minio_client.make_bucket(bucket_name=CONFIG.MINIO_DEV_TESTS_BUCKET)
+ except Exception as e:
+ # Bucket might have been created by another process
+ if not self.minio_client.bucket_exists(CONFIG.MINIO_DEV_TESTS_BUCKET):
+ raise # Re-raise if bucket still doesn't existBy the way, this change seems completely unrelated to fixing the exceptiongroup dependency for Py3.10. What's the connection here? Did the dependency fix somehow break your test environment?
I'll need to gather more context to properly validate this review. Let me check the codebase and verify the concerns raised.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the actual file content and surrounding context
cat python/tests/test_minio_bucket.py | head -30
Length of output: 1350
🏁 Script executed:
#!/bin/bash
# Search for other bucket creation or bucket_exists patterns
rg -n "bucket_exists|make_bucket" --type py -C 3Length of output: 895
🏁 Script executed:
#!/bin/bash
# Check PR details and git context
git log --oneline -20 --all 2>/dev/null | head -20 || echo "Git context unavailable"
git show --stat 2>/dev/null | head -50 || echo "Git show unavailable"Length of output: 1777
🏁 Script executed:
#!/bin/bash
# Search for MINIO in the codebase to understand overall usage
rg -n "minio_client" --type py -C 2 | head -50Length of output: 3951
---
🌐 Web query:
```web
Minio Python SDK make_bucket exception when bucket already exists
💡 Result:
Use bucket_exists before calling make_bucket — or catch S3Error and ignore the "already exists" codes.
Example 1 — preferred (check then create):
from minio import Minio
from minio.error import S3Error
client = Minio("play.min.io", access_key="...", secret_key="...")
if not client.bucket_exists("my-bucket"):
client.make_bucket("my-bucket")
(else: bucket already exists)
(see MinIO Python quickstart). [1]
Example 2 — idempotent-create via exception handling:
from minio import Minio
from minio.error import S3Error
client = Minio("play.min.io", access_key="...", secret_key="...")
try:
client.make_bucket("my-bucket")
except S3Error as exc:
# ignore if bucket already exists (S3 may return "BucketAlreadyOwnedByYou" or "BucketAlreadyExists")
if exc.code in ("BucketAlreadyOwnedByYou", "BucketAlreadyExists"):
pass
else:
raise
(see MinIO Python examples / S3Error usage). [1][2]
References:
[1] MinIO Python quickstart / examples (bucket_exists + make_bucket).
[2] minio-py project README / examples (S3Error usage).
Separate commit with solid defensive approach—error handling is optional, not required.
This bucket creation is actually separate from the exceptiongroup fix (commit 4aa89a6 vs 119905d), and the check-then-create pattern is the preferred approach per Minio's own documentation. The code is sound.
On the TOCTOU concern: theoretically valid, but test setUp typically runs serially per instance. Parallel test workers would each have their own setUp, so collision risk is minimal. A realistic worry, but low probability.
On error handling: not necessary. The Minio SDK shows two patterns—check-then-create (what you have) and try-except-catch. Both work. Your current code is cleaner for the normal case.
If you want defensive error handling anyway, the pattern is clearer:
try:
if not self.minio_client.bucket_exists(CONFIG.MINIO_DEV_TESTS_BUCKET):
self.minio_client.make_bucket(bucket_name=CONFIG.MINIO_DEV_TESTS_BUCKET)
except S3Error as e:
if e.code not in ("BucketAlreadyOwnedByYou", "BucketAlreadyExists"):
raiseBut honestly? The simple check-then-create you have is production-grade. Ship it.
🤖 Prompt for AI Agents
python/tests/test_minio_bucket.py lines 19-20: leave the existing
check-then-create bucket logic as-is (this is acceptable and should be a
separate commit from the exceptiongroup fix); if you prefer an optional
defensive change, wrap the bucket existence/make logic in a try/except that
catches S3Error and suppresses only the "BucketAlreadyOwnedByYou" and
"BucketAlreadyExists" codes, re-raising any other errors, then commit that
change separately.
Title
<type>:<short summary>Ticket / Task
<URL>What & Why
"Builds X so that Y can Z" – two sentences max.
Scope
Checklist (self-review)
See Section 5.
Risk / Impact
Screenshots / Logs / Benchmarks
(only if relevant)
Author Self-Review Checklist
pytest -q/mvn testclean locally.git revertwithout dependency hell.python/scripts/run_mypy.batlocally with no critical errorsResources: