[WEB-4013]chore: correct live url#7014
Conversation
WalkthroughThe changes update the way URLs are constructed and normalized in the background task for copying S3 objects. The code in Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant copy_s3_object.py
participant utils/url.py
Caller->>copy_s3_object.py: Initiate S3 object copy
copy_s3_object.py->>copy_s3_object.py: Get settings.LIVE_URL
copy_s3_object.py->>utils/url.py: normalize_url_path(live_url + "/convert-document/")
utils/url.py-->>copy_s3_object.py: Return normalized URL
copy_s3_object.py->>copy_s3_object.py: Send POST request to normalized URL
copy_s3_object.py-->>Caller: Return response/result
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apiserver/plane/utils/url.py (2)
60-60: Line exceeds character limit.This docstring line exceeds the 88-character limit according to the linter.
- Normalize the path component of a URL by replacing multiple consecutive slashes with a single slash. + Normalize the path component of a URL by replacing multiple consecutive slashes with a + single slash.🧰 Tools
🪛 Ruff (0.8.2)
60-60: Line too long (104 > 88)
(E501)
62-62: Line exceeds character limit.This docstring line exceeds the 88-character limit according to the linter.
- This function preserves the protocol, domain, query parameters, and fragments of the URL, + This function preserves the protocol, domain, query parameters, and fragments of the URL, + only modifying the path portion to ensure there are no duplicate slashes. - only modifying the path portion to ensure there are no duplicate slashes.🧰 Tools
🪛 Ruff (0.8.2)
62-62: Line too long (93 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apiserver/plane/bgtasks/copy_s3_object.py(3 hunks)apiserver/plane/utils/url.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apiserver/plane/bgtasks/copy_s3_object.py (1)
apiserver/plane/utils/url.py (1)
normalize_url_path(58-79)
🪛 Ruff (0.8.2)
apiserver/plane/utils/url.py
60-60: Line too long (104 > 88)
(E501)
62-62: Line too long (93 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
apiserver/plane/utils/url.py (1)
58-79: Well-designed URL normalization function.The
normalize_url_pathfunction elegantly handles normalizing URL paths by collapsing multiple consecutive slashes. Usingurlparseandurlunparseensures other URL components remain untouched.🧰 Tools
🪛 Ruff (0.8.2)
60-60: Line too long (104 > 88)
(E501)
62-62: Line too long (93 > 88)
(E501)
apiserver/plane/bgtasks/copy_s3_object.py (3)
15-15: Good import change.Appropriately imported the new
normalize_url_pathfunction to replace the previous URL joining mechanism.
72-77: Simplified URL handling logic.The code now uses a more straightforward approach for handling the live URL. The
normalize_url_pathfunction ensures that any potential double slashes from concatenation are properly normalized.
76-76:Details
✅ Verification successful
Verify URL construction consistency.
Make sure this URL construction behaves consistently with the previous implementation, especially for the test scenario mentioned in the PR objectives (checking for duplicate pages).
🏁 Script executed:
#!/bin/bash # Check sample URLs to ensure normalization works as expected # Test with different potential LIVE_URL formats echo "Testing URL normalization with different URL formats:" echo "-------------------------------------------------" # Test with trailing slash python3 -c "from plane.utils.url import normalize_url_path; print(normalize_url_path('https://example.com/' + '/convert-document/'))" # Test without trailing slash python3 -c "from plane.utils.url import normalize_url_path; print(normalize_url_path('https://example.com' + '/convert-document/'))" # Test with multiple slashes python3 -c "from plane.utils.url import normalize_url_path; print(normalize_url_path('https://example.com///' + '///convert-document/'))"Length of output: 1027
🏁 Script executed:
#!/bin/bash # Locate and display normalize_url_path implementation echo "Finding normalize_url_path definition..." RG_OUT=$(rg "def normalize_url_path" -n) echo "$RG_OUT" FILE=$(echo "$RG_OUT" | head -n1 | cut -d: -f1) LINE=$(echo "$RG_OUT" | head -n1 | cut -d: -f2) echo -e "\nShowing implementation in $FILE (lines $LINE–$(($LINE+30))):" sed -n "${LINE},$((LINE+30))p" "$FILE"Length of output: 1653
🏁 Script executed:
#!/bin/bash # Display context around the URL construction in copy_s3_object.py echo "Showing lines 1–120 of apiserver/plane/bgtasks/copy_s3_object.py for context:" sed -n '1,120p' apiserver/plane/bgtasks/copy_s3_object.py # Locate live_url assignment echo -e "\nSearching for live_url definition in the file:" rg -n "live_url" -n apiserver/plane/bgtasks/copy_s3_object.pyLength of output: 4912
URL construction is consistent with previous behavior.
The
normalize_url_pathfunction collapses any duplicate slashes in the path segment, so whethersettings.LIVE_URLends with a slash or not (or even contains multiple slashes), the resulting URL will always behttps://…/convert-document/. No changes are needed here.
2 participants
Description
chore: correct live url
Type of Change
Test Scenarios
References
WEB-4013
Summary by CodeRabbit
New Features
Bug Fixes