feat: expose str_to_datetime jinja macro
#351
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughWalkthroughThe pull request updates copyright notices in several interpolation modules from 2023 to 2025. Additionally, in the macros module, a private function ( Changes
Sequence Diagram(s)sequenceDiagram
participant TS as Test Suite
participant F as str_to_datetime Function
TS->>F: Call str_to_datetime(input_string)
F-->>TS: Return datetime object
Would you like any additional details or modifications to this diagram, wdyt? ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
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: 2
🧹 Nitpick comments (1)
airbyte_cdk/sources/declarative/interpolation/macros.py (1)
69-74: Great job on making the datetime parsing function public! 🎉The function is well-documented and handles timezone information appropriately. Since this is now part of the public API, would you consider adding docstring with usage examples? wdyt?
def str_to_datetime(s: str) -> datetime.datetime: + """ + Converts a string to a datetime object, assuming UTC if no timezone is specified. + + Usage: + `"{{ str_to_datetime('2023-01-01T00:00:00Z') }}"` + `"{{ str_to_datetime('2023-01-01') }}"` # Assumes UTC + + :param s: datetime string in ISO format + :return: datetime object in UTC timezone + """ parsed_date = parser.isoparse(s)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
airbyte_cdk/sources/declarative/interpolation/__init__.py(1 hunks)airbyte_cdk/sources/declarative/interpolation/filters.py(1 hunks)airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py(1 hunks)airbyte_cdk/sources/declarative/interpolation/interpolated_mapping.py(1 hunks)airbyte_cdk/sources/declarative/interpolation/interpolated_nested_mapping.py(1 hunks)airbyte_cdk/sources/declarative/interpolation/interpolated_string.py(1 hunks)airbyte_cdk/sources/declarative/interpolation/interpolation.py(1 hunks)airbyte_cdk/sources/declarative/interpolation/jinja.py(1 hunks)airbyte_cdk/sources/declarative/interpolation/macros.py(4 hunks)unit_tests/sources/declarative/interpolation/test_macros.py(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- airbyte_cdk/sources/declarative/interpolation/init.py
- airbyte_cdk/sources/declarative/interpolation/interpolated_string.py
- airbyte_cdk/sources/declarative/interpolation/jinja.py
- airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py
- airbyte_cdk/sources/declarative/interpolation/interpolation.py
- airbyte_cdk/sources/declarative/interpolation/filters.py
- airbyte_cdk/sources/declarative/interpolation/interpolated_nested_mapping.py
- airbyte_cdk/sources/declarative/interpolation/interpolated_mapping.py
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Check: 'source-shopify' (skip=false)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
🔇 Additional comments (1)
airbyte_cdk/sources/declarative/interpolation/macros.py (1)
175-175: LGTM on adding to _macros_list! 👍Good catch on adding the newly public function to the macros list.
natikgadzhi
approved these changes
Feb 20, 2025
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
unit_tests/sources/declarative/interpolation/test_macros.py (1)
125-148: Consider adding more test cases for better coverage.The current test cases look good, but would you consider adding tests for:
- Invalid date strings to verify error handling
- Different timezone offsets (e.g., +0530, -0800)
- Microseconds precision
wdyt?@@ -125,6 +125,21 @@ @pytest.mark.parametrize( "test_name, input_value, expected_output", [ + ( + "test_invalid_date", + "invalid-date", + pytest.raises(ValueError, match="Invalid date format") + ), + ( + "test_datetime_with_timezone_offset", + "2022-01-01T13:45:30+05:30", + datetime.datetime(2022, 1, 1, 8, 15, 30, tzinfo=datetime.timezone.utc), + ), + ( + "test_datetime_with_microseconds", + "2022-01-01T13:45:30.123456Z", + datetime.datetime(2022, 1, 1, 13, 45, 30, 123456, tzinfo=datetime.timezone.utc), + ), ( "test_basic_date", "2022-01-14",airbyte_cdk/sources/declarative/declarative_component_schema.yaml (1)
3747-3754: Consider adding more examples to improve documentation.The macro documentation looks good, but would you consider adding more examples to demonstrate different input formats? For instance:
- Datetime with time
- Datetime with timezone
- Datetime with microseconds
wdyt?examples: - "{{ str_to_datetime('2022-01-14') }}" + - "{{ str_to_datetime('2022-01-01 13:45:30') }}" + - "{{ str_to_datetime('2022-01-01T13:45:30+00:00') }}" + - "{{ str_to_datetime('2022-01-01T13:45:30.123456Z') }}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(1 hunks)airbyte_cdk/sources/declarative/interpolation/macros.py(4 hunks)unit_tests/sources/declarative/interpolation/test_macros.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- airbyte_cdk/sources/declarative/interpolation/macros.py
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Check: 'source-pokeapi' (skip=false)
- GitHub Check: Check: 'source-amplitude' (skip=false)
- GitHub Check: Check: 'source-shopify' (skip=false)
- GitHub Check: Check: 'source-hardcoded-records' (skip=false)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
unit_tests/sources/declarative/interpolation/test_macros.py (1)
163-166: LGTM! Good error handling test.The error case test ensures robustness against invalid inputs.
Would you like to add more specific error cases? For example:
- Empty string
- None input
- Malformed but date-like strings (e.g., "2022-13-45")
wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(1 hunks)unit_tests/sources/declarative/interpolation/test_macros.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- airbyte_cdk/sources/declarative/declarative_component_schema.yaml
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Check: 'source-pokeapi' (skip=false)
- GitHub Check: Check: 'source-amplitude' (skip=false)
- GitHub Check: Check: 'source-shopify' (skip=false)
- GitHub Check: Check: 'source-hardcoded-records' (skip=false)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
unit_tests/sources/declarative/interpolation/test_macros.py (2)
2-2: LGTM!The copyright notice has been updated appropriately.
125-161: Great test coverage! 🎯The test cases comprehensively cover various date string formats and edge cases. I particularly like the inclusion of timezone offsets and microseconds precision.
However, I noticed from the past review comment that there might be a signature mismatch with the implementation. The implementation in
macros.pyaccepts only(s: str), but some test cases seem to expect format parameters. Should we:
- Update the implementation to support custom formats, or
- Remove format-related test cases?
wdyt?
rpopov
added a commit
to rpopov/airbyte-python-cdk
that referenced
this pull request
Mar 5, 2025
* main: fix: update cryptography package to latest version to address CVE (airbytehq#377) fix: (CDK) (HttpRequester) - Make the `HttpRequester.path` optional (airbytehq#370) feat: improved custom components handling (airbytehq#350) feat: add microseconds timestamp format (airbytehq#373) fix: Replace Unidecode with anyascii for permissive license (airbytehq#367) feat: add IncrementingCountCursor (airbytehq#346) feat: (low-code cdk) datetime format with milliseconds (airbytehq#369) fix: (CDK) (AsyncRetriever) - Improve UX on variable naming and interpolation (airbytehq#368) fix: (CDK) (AsyncRetriever) - Add the `request` and `response` to each `async` operations (airbytehq#356) fix: (CDK) (ConnectorBuilder) - Add `auxiliary requests` to slice; support `TestRead` for AsyncRetriever (part 1/2) (airbytehq#355) feat(concurrent perpartition cursor): Add parent state updates (airbytehq#343) fix: update csv parser for builder compatibility (airbytehq#364) feat(low-code cdk): add interpolation for limit field in Rate (airbytehq#353) feat(low-code cdk): add AbstractStreamFacade processing as concurrent streams in declarative source (airbytehq#347) fix: (CDK) (CsvParser) - Fix the `\\` escaping when passing the `delimiter` from Builder's UI (airbytehq#358) feat: expose `str_to_datetime` jinja macro (airbytehq#351) fix: update CDK migration for 6.34.0 (airbytehq#348) feat: Removes `stream_state` interpolation from CDK (airbytehq#320) fix(declarative): Pass `extra_fields` in `global_substream_cursor` (airbytehq#195) feat(concurrent perpartition cursor): Refactor ConcurrentPerPartitionCursor (airbytehq#331) feat(HttpMocker): adding support for PUT requests and bytes responses (airbytehq#342) chore: use certified source for manifest-only test (airbytehq#338) feat: check for request_option mapping conflicts in individual components (airbytehq#328) feat(file-based): sync file acl permissions and identities (airbytehq#260) fix: (CDK) (Connector Builder) - refactor the `MessageGrouper` > `TestRead` (airbytehq#332) fix(low code): Fix missing cursor for ClientSideIncrementalRecordFilterDecorator (airbytehq#334) feat(low-code): Add API Budget (airbytehq#314) chore(decoder): clean decoders and make csvdecoder available (airbytehq#326)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
source-tiktok-marketingneeds access to thestr_to_datetimejinja macro as related to this issue. This PR exposes this macro for public use and adds appropriate tests.Summary by CodeRabbit