-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[WEB-4805] fix: upgraded psycopgy packages to fix linting and removed unused imports #7735
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.
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
Conversation
WalkthroughRemoved unused imports, added lint-suppression inline comments, reformatted several docstrings and examples, adjusted one test string literal, removed one test utility import, and bumped psycopg and lxml versions in requirements. No functional or control-flow changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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 |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR upgrades the psycopg PostgreSQL adapter packages from version 3.1.18 to 3.2.9 to fix linting issues, and removes unused imports across multiple files to improve code cleanliness.
- Upgraded psycopg, psycopg-binary, and psycopg-c packages to version 3.2.9
- Removed unused imports from test files, view files, serializers, and API endpoints
- Fixed string formatting in test file to remove f-string prefix where no interpolation was used
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/api/requirements/base.txt | Upgraded psycopg packages from 3.1.18 to 3.2.9 |
| apps/api/plane/tests/unit/utils/test_url.py | Removed unused get_url_components import |
| apps/api/plane/tests/unit/bg_tasks/test_copy_s3_objects.py | Fixed unnecessary f-string usage |
| apps/api/plane/tests/contract/api/test_labels.py | Removed unused IntegrityError import |
| apps/api/plane/app/views/workspace/base.py | Removed unused Django decorator imports |
| apps/api/plane/app/views/project/base.py | Removed unused IntegrityError and serializers imports |
| apps/api/plane/app/views/page/base.py | Removed unused base64 import |
| apps/api/plane/app/serializers/project.py | Removed unused validate_binary_data import |
| apps/api/plane/app/serializers/issue.py | Removed unused lxml.html import |
| apps/api/plane/app/serializers/draft.py | Removed unused lxml.html import |
| apps/api/plane/api/views/state.py | Removed unused OpenApiExample import |
| apps/api/plane/api/views/project.py | Removed unused OpenApiExample import |
| apps/api/plane/api/views/module.py | Removed unused OpenApiExample and parameter imports |
| apps/api/plane/api/views/issue.py | Removed unused drf-spectacular imports and parameters |
| apps/api/plane/api/views/asset.py | Removed unused OpenApiTypes import |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/api/plane/utils/paginator.py (2)
293-301: Bug: QuerySet truthiness check will raise in Django.
if results:on a QuerySet is invalid (“truth value of a QuerySet is ambiguous”) and can raise at runtime. Use.exists().- if results: + if results.exists(): max_hits = math.ceil( queryset.values(self.group_by_field_name) .annotate(count=Count("id", filter=self.count_filter, distinct=True)) .order_by("-count")[0]["count"] / limit ) else: max_hits = 0
513-521: Bug: QuerySet truthiness check will raise in Django.Same as GroupedOffsetPaginator; use
.exists().- if results: + if results.exists(): max_hits = math.ceil( queryset.values(self.group_by_field_name) .annotate(count=Count("id", filter=self.count_filter, distinct=True)) .order_by("-count")[0]["count"] / limit ) else: max_hits = 0apps/api/plane/api/views/asset.py (4)
109-111: Fix lint: E501 long docstring line (pipeline failing).Wrap the long line to ≤88 chars.
- Create a presigned URL for uploading user profile assets (avatar or cover image). - This endpoint generates the necessary credentials for direct S3 upload. + Create a presigned URL for uploading user profile assets (avatar or cover + image). This endpoint generates the necessary credentials for direct S3 + upload.
205-206: Fix lint: E501 long docstring line (pipeline failing).Wrap to satisfy flake8’s 88-char limit.
- This endpoint should be called after completing the S3 upload to mark the asset as uploaded. + This endpoint should be called after completing the S3 upload to mark the + asset as uploaded.
136-143: Error message contradicts allowed types.Allowed list includes WEBP and GIF, but the message says “Only JPEG and PNG…”. Align the message with the actual validation.
- { - "error": "Invalid file type. Only JPEG and PNG files are allowed.", - "status": False, - }, + { + "error": "Invalid file type. Allowed types: JPEG, PNG, WEBP, GIF.", + "status": False, + },
313-320: Same mismatch for server endpoint.Update the error message to match
allowed_types.- { - "error": "Invalid file type. Only JPEG and PNG files are allowed.", - "status": False, - }, + { + "error": "Invalid file type. Allowed types: JPEG, PNG, WEBP, GIF.", + "status": False, + },
🧹 Nitpick comments (3)
apps/api/plane/utils/paginator.py (2)
224-225: Fix grammar in comment.Use “these are” instead of “this are” and tighten wording.
- # Set the count filter - this are extra filters that need to be passed - # to calculate the counts with the filters + # Set the count filter - these are extra filters used to calculate counts + # with the given filters
438-440: Fix grammar in comment.Same issue as above; update for clarity.
- # Set the count filter - this are extra filters that need - # to be passed to calculate the counts with the filters + # Set the count filter - these are extra filters used + # to calculate counts with the given filtersapps/api/plane/api/views/asset.py (1)
128-136: De-duplicate allowed MIME types across endpoints.Both endpoints hardcode the same list. Consider centralizing (e.g., use
settings.ATTACHMENT_MIME_TYPESfiltered toimage/*or a dedicatedUSER_IMAGE_MIME_TYPES) to avoid drift.Also applies to: 305-313
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/api/plane/api/serializers/base.py(1 hunks)apps/api/plane/api/serializers/issue.py(1 hunks)apps/api/plane/api/serializers/module.py(3 hunks)apps/api/plane/api/views/asset.py(2 hunks)apps/api/plane/utils/openapi/examples.py(4 hunks)apps/api/plane/utils/openapi/parameters.py(1 hunks)apps/api/plane/utils/openapi/responses.py(1 hunks)apps/api/plane/utils/paginator.py(2 hunks)apps/api/plane/utils/url.py(3 hunks)
✅ Files skipped from review due to trivial changes (7)
- apps/api/plane/api/serializers/issue.py
- apps/api/plane/api/serializers/base.py
- apps/api/plane/api/serializers/module.py
- apps/api/plane/utils/url.py
- apps/api/plane/utils/openapi/parameters.py
- apps/api/plane/utils/openapi/responses.py
- apps/api/plane/utils/openapi/examples.py
🧰 Additional context used
🪛 GitHub Actions: Build and lint API
apps/api/plane/api/views/asset.py
[error] 109-109: E501 Line too long (89 > 88).
[error] 205-205: E501 Line too long (100 > 88).
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/api/plane/api/views/asset.py (1)
11-11: Import cleanup LGTM.Removing the unused
OpenApiTypesis appropriate; no runtime impact.
… unused imports (makeplane#7735) * chore: update psycopg dependencies to version 3.2.9 in base requirements * refactor: clean up unused imports across multiple files * chore: update lxml dependency to version 6.0.0 in base requirements * style: improve code readability by breaking long lines into multiple lines across several files * style: enhance readability by breaking long lines in ModuleSerializer docstring
Description
fix: upgraded psycopgy packages to fix linting and removed unused imports
Type of Change
Test Scenarios
References
WEB-4805
Summary by CodeRabbit