Allow fingerprint duration more than ~9 hours #158
Conversation
WalkthroughThis pull request updates two main areas. First, it adjusts the validation logic for the Changes
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🧰 Additional context used🪛 Ruff (0.8.2)alembic/versions/468b44482655_change_fingerprint_length_to_int4.py10-10: Remove unused import: (F401) 19-19: Missing return type annotation for public function Add return type annotation: (ANN201) 19-19: Missing type annotation for function argument (ANN001) 20-20: Use format specifiers instead of percent format Replace with format specifiers (UP031) 23-23: Missing return type annotation for public function Add return type annotation: (ANN201) 23-23: Missing type annotation for function argument (ANN001) 24-24: Use format specifiers instead of percent format Replace with format specifiers (UP031) 27-27: Missing return type annotation for public function Add return type annotation: (ANN201) 33-33: Missing return type annotation for public function Add return type annotation: (ANN201) 39-39: Missing return type annotation for public function Add return type annotation: (ANN201) 45-45: Missing return type annotation for public function Add return type annotation: (ANN201) 51-51: Missing return type annotation for public function Add return type annotation: (ANN201) 60-60: Missing return type annotation for public function Add return type annotation: (ANN201) alembic/versions/d7e8b1c80561_change_submission_length_to_int4.py10-10: Remove unused import: (F401) 19-19: Missing return type annotation for public function Add return type annotation: (ANN201) 19-19: Missing type annotation for function argument (ANN001) 20-20: Use format specifiers instead of percent format Replace with format specifiers (UP031) 23-23: Missing return type annotation for public function Add return type annotation: (ANN201) 23-23: Missing type annotation for function argument (ANN001) 24-24: Use format specifiers instead of percent format Replace with format specifiers (UP031) 27-27: Missing return type annotation for public function Add return type annotation: (ANN201) 33-33: Missing return type annotation for public function Add return type annotation: (ANN201) 39-39: Missing return type annotation for public function Add return type annotation: (ANN201) 48-48: Missing return type annotation for public function Add return type annotation: (ANN201) 57-57: Missing return type annotation for public function Add return type annotation: (ANN201) 63-63: Missing return type annotation for public function Add return type annotation: (ANN201) ⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Can't merge this yet, as there is no good way to change the type in PostgreSQL, these are huge tables, can't have them locked for days while they are being rewritten. |
|
I'll most likely time this with upgrading all databases to PostgreSQL 17, as that will require dump/restore. |
There was a problem hiding this comment.
PR Overview
This PR extends the allowed duration range and updates related database migration scripts and table definitions to support longer durations.
- Updates migration scripts to alter the "length" column from SMALLINT to Integer for fingerprints and submissions.
- Adjusts table definitions in acoustid/tables.py to reflect the new integer type.
- Modifies the duration validation in the API to use an extended upper bound.
Reviewed Changes
| File | Description |
|---|---|
| alembic/versions/468b44482655_change_fingerprint_length_to_int4.py | Alters the fingerprint length column type to Integer and includes engine-specific no-op migration functions. |
| alembic/versions/d7e8b1c80561_change_submission_length_to_int4.py | Alters the submission length column type to Integer with similar no-op migration functions for unused engines. |
| acoustid/tables.py | Updates table definitions to use Integer for the "length" column. |
| acoustid/api/v2/init.py | Extends the allowed duration range by updating the upper limit in the duration validation check. |
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
alembic/versions/468b44482655_change_fingerprint_length_to_int4.py:29
- [nitpick] The auto-generated upgrade_app function is currently a no-op; please add a clarifying comment to indicate that no migration steps are necessary for this engine to avoid future confusion.
pass
acoustid/api/v2/init.py:910
- [nitpick] Consider enhancing the error message for InvalidDurationError by including the expected allowed range to assist in debugging and user feedback.
if p["duration"] <= 0 or p["duration"] > 0x7FFFFFFF:
Summary by CodeRabbit
New Features
Chores