feat: add sanitize_url jinja macro#571
Conversation
📝 WalkthroughWalkthroughA new macro function, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant JinjaContext
participant sanitize_url
User->>JinjaContext: Uses {{ sanitize_url(value) }} in template
JinjaContext->>sanitize_url: Calls sanitize_url(value)
sanitize_url-->>JinjaContext: Returns URL-encoded string
JinjaContext-->>User: Renders template with sanitized value
✨ 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 (
|
![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 (1)
airbyte_cdk/sources/declarative/interpolation/macros.py (1)
186-194: Consider some refinements to improve clarity and consistency, wdyt?A few suggestions for the function implementation:
Naming consistency: The function is called
sanitize_urlbut it works on any string (as shown in the tests with "hello world"). Wouldsanitize_stringorurl_encodebe more accurate?Simplify implementation: The intermediate
sanitization_strategyvariable seems unnecessary. Could we directly returnquote_plus(value)?Fix docstring grammar: "Sanitizes a value by via urllib.parse.quote_plus" has redundant words. How about "Sanitizes a value using urllib.parse.quote_plus"?
Here's a potential refinement:
-def sanitize_url(value: str) -> str: +def url_encode(value: str) -> str: """ - Sanitizes a value by via urllib.parse.quote_plus + URL-encodes a string using urllib.parse.quote_plus Usage: - `"{{ sanitize_url('https://example.com/path?query=value') }}"` + `"{{ url_encode('https://example.com/path?query=value') }}"` """ - sanitization_strategy = quote_plus - return sanitization_strategy(value) + return quote_plus(value)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airbyte_cdk/sources/declarative/interpolation/macros.py(3 hunks)unit_tests/sources/declarative/interpolation/test_macros.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
unit_tests/sources/declarative/interpolation/test_macros.py (1)
airbyte_cdk/sources/declarative/interpolation/macros.py (1)
sanitize_url(186-194)
🔇 Additional comments (3)
airbyte_cdk/sources/declarative/interpolation/macros.py (2)
9-9: Import addition looks good!The import of
quote_plusis correctly placed and necessary for the new functionality.
208-208: Function registration is correct!Properly added to the
_macros_listfollowing the existing pattern.unit_tests/sources/declarative/interpolation/test_macros.py (1)
227-251: Excellent test coverage!The parameterized tests cover the key scenarios well:
- Basic URL encoding
- Space handling (converted to
+)- Special character encoding (
&,+,#)- Non-URL string handling
The expected outputs match
quote_plusbehavior correctly. One small question: would it be worth adding an edge case test for empty string input, wdyt? Though the current coverage is quite comprehensive.
2 participants
What
sanitizejinja macro which defaults to urllib.parsequote_plussanitization.Summary by CodeRabbit
New Features
Tests