-
Notifications
You must be signed in to change notification settings - Fork 30
feat: add suggestions for datetime based cursor fields #721
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
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@lmossman/add-suggestions-for-datetime-fields#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch lmossman/add-suggestions-for-datetime-fields Helpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
📝 WalkthroughWalkthroughAdds UI-only suggestion arrays to datetime/duration fields in the declarative component schema (DatetimeBasedCursor and MinMaxDatetime). No behavioral or control-flow changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Would you like to also add aligned examples or enum hints for these fields across the schema to keep consistency, wdyt? Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (5)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (5)
990-1002
: Consider adding a common "space + %z" variant to suggestionsMany APIs return "YYYY-MM-DD HH:MM:SS±HH:MM" without the literal T. You already include the T+%z variant and a hard-coded "+00:00" literal; adding the space+%z pattern would round out common cases. Also, since these suggestion lists are duplicated in multiple places, would you like to centralize them via a shared anchor/$ref to avoid drift over time, wdyt?
Apply this minimal addition:
suggestions: - "%Y-%m-%d" - "%Y-%m-%d %H:%M:%S" - "%Y-%m-%dT%H:%M:%S" - "%Y-%m-%dT%H:%M:%SZ" - "%Y-%m-%dT%H:%M:%S%z" - "%Y-%m-%dT%H:%M:%S.%fZ" - "%Y-%m-%dT%H:%M:%S.%f%z" - "%Y-%m-%d %H:%M:%S.%f+00:00" + - "%Y-%m-%d %H:%M:%S%z" - "%s" - "%ms" - "%s_as_float"
1076-1087
: Mirror the "space + %z" pattern in outgoing datetime_format suggestionsTo keep parity with cursor formats and common API expectations, would you like to include "%Y-%m-%d %H:%M:%S%z" here as well, wdyt?
suggestions: - "%Y-%m-%d" - "%Y-%m-%d %H:%M:%S" - "%Y-%m-%dT%H:%M:%S" - "%Y-%m-%dT%H:%M:%SZ" - "%Y-%m-%dT%H:%M:%S%z" - "%Y-%m-%dT%H:%M:%S.%fZ" - "%Y-%m-%dT%H:%M:%S.%f%z" - "%Y-%m-%d %H:%M:%S.%f+00:00" + - "%Y-%m-%d %H:%M:%S%z" - "%s" - "%ms" - "%s_as_float"
1103-1108
: Add a couple of mid-range ISO 8601 durations frequently used in practicePT30M and PT5M are very common granularity values for hourly/minutely timestamps. Including them may save users a few keystrokes. Interested in adding them, wdyt?
suggestions: - "PT0.000001S" - "PT0.001S" - "PT1S" + - "PT5M" - "PT1M" + - "PT30M" - "PT1H" - "P1D"
1168-1173
: Expand step suggestions to cover finer-grained windowsMany sources slice by 5-, 15-, or 30-minute windows. Would you like to include these as additional suggestions for step to better match real-world configs, wdyt?
suggestions: + - "PT5M" + - "PT15M" + - "PT30M" - "PT1H" - "P1D" - "P1W" - "P1M" - "P1Y"
2845-2855
: Keep MinMaxDatetime suggestions aligned with the cursor formatsFor consistency with DatetimeBasedCursor suggestions (and common API formats), consider adding the space+%z variant here too. This helps when configs use non-ISO "space" separators. Shall we add it, wdyt?
suggestions: - "%Y-%m-%d" - "%Y-%m-%d %H:%M:%S" - "%Y-%m-%dT%H:%M:%S" - "%Y-%m-%dT%H:%M:%SZ" - "%Y-%m-%dT%H:%M:%S%z" - "%Y-%m-%dT%H:%M:%S.%fZ" - "%Y-%m-%dT%H:%M:%S.%f%z" - "%Y-%m-%d %H:%M:%S.%f+00:00" + - "%Y-%m-%d %H:%M:%S%z" - "%s" - "%ms" - "%s_as_float"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
(5 hunks)
⏰ 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). (9)
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-intercom
- GitHub Check: Check: source-shopify
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (Fast)
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.
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 enhances the Connector Builder UI by adding common datetime format suggestions to various DatetimeBasedCursor fields to improve usability and reduce configuration errors. The changes provide users with helpful examples for common date/time patterns without altering any runtime behavior.
- Adds UI suggestions for common datetime formats (ISO 8601, YYYY-MM-DD, epoch, etc.) across multiple cursor field types
- Adds UI suggestions for ISO 8601 duration values for step/duration inputs
- Improves user experience by providing pre-populated examples for frequently confusing configuration fields
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
🧹 Nitpick comments (5)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (5)
990-1002
: Nice UX win adding suggestions to cursor_datetime_formats. Consider rounding out HTTP/RFC and millisecond-precision variants.The list covers the most common cases. To reduce back-and-forth for APIs that return RFC 1123 dates (very common in HTTP headers) or strict millisecond precision, would you consider adding these as well?
- RFC 1123: "%a, %d %b %Y %H:%M:%S %z"
- Milliseconds placeholder variants you already document: "%Y-%m-%dT%H:%M:%S.%_msZ" and "%Y-%m-%dT%H:%M:%S.%_ms%z"
Also, since this field is array, can you confirm the Builder reads suggestions when placed at the array node (not under items)? If the platform expects it under the same node you used here, we're good; otherwise, we might need to move it. wdyt?
Apply diff (appends new suggestions at the end):
- "%ms" - "%s_as_float" + - "%a, %d %b %Y %H:%M:%S %z" + - "%Y-%m-%dT%H:%M:%S.%_msZ" + - "%Y-%m-%dT%H:%M:%S.%_ms%z"
1076-1087
: Outgoing datetime_format suggestions look good. Maybe include the default format and a couple widely used variants.Given the documented default is "%Y-%m-%dT%H:%M:%S.%f%z", it can help to surface it first in suggestions so users pick it quickly. I’d also add the RFC 1123 and strict millisecond formats you already document.
Would you like to add these and place the default at the top? wdyt?
Apply diff:
- suggestions: - - "%Y-%m-%d" + suggestions: + - "%Y-%m-%dT%H:%M:%S.%f%z" # default + - "%Y-%m-%d" - "%Y-%m-%d %H:%M:%S" - "%Y-%m-%dT%H:%M:%S" - "%Y-%m-%dT%H:%M:%SZ" - "%Y-%m-%dT%H:%M:%S%z" - "%Y-%m-%dT%H:%M:%S.%fZ" - "%Y-%m-%dT%H:%M:%S.%f%z" - "%Y-%m-%d %H:%M:%S.%f+00:00" - "%s" - "%ms" - "%s_as_float" + - "%a, %d %b %Y %H:%M:%S %z" + - "%Y-%m-%dT%H:%M:%S.%_msZ" + - "%Y-%m-%dT%H:%M:%S.%_ms%z"
1102-1108
: Granularity set is helpful; consider common “in-between” durations.Many APIs paginate/limit at 30s or 15m boundaries. Adding them reduces guesswork for users mapping their format to minimum increment. Additions to consider: PT30S, PT15M. wdyt?
Apply diff:
suggestions: - "PT0.000001S" - "PT0.001S" - "PT1S" + - "PT30S" - "PT1M" + - "PT15M" - "PT1H" - "P1D"
1167-1172
: Step suggestions cover the essentials; adding a few popular intervals could help.In practice we often see half-day and month-ish windows. Would you consider appending PT6H, PT12H, P7D, and P30D to the list to save typing? wdyt?
Apply diff:
suggestions: - "PT1H" - "P1D" - "P1W" - "P1M" - "P1Y" + - "PT6H" + - "PT12H" + - "P7D" + - "P30D"
2845-2855
: Good call adding suggestions to MinMaxDatetime.datetime_format; mirror the defaults and HTTP-date too?To align with DatetimeBasedCursor and make common cases 1-click, could we:
- Include the documented default "%Y-%m-%dT%H:%M:%S.%f%z" at the top,
- Add RFC 1123 ("%a, %d %b %Y %H:%M:%S %z"),
- Add the strict millisecond placeholders ("%_ms") variants?
Also, for a future follow-up, would you want suggestions on MinMaxDatetime.min_datetime/max_datetime themselves (e.g., "{{ now_utc() }}", "{{ today_utc().strftime('%Y-%m-%d') }}", and an ISO example) and on lookback_window/expiration_duration to keep the UX consistent across datetime/duration inputs? wdyt?
Apply diff:
- suggestions: - - "%Y-%m-%d" + suggestions: + - "%Y-%m-%dT%H:%M:%S.%f%z" # default + - "%Y-%m-%d" - "%Y-%m-%d %H:%M:%S" - "%Y-%m-%dT%H:%M:%S" - "%Y-%m-%dT%H:%M:%SZ" - "%Y-%m-%dT%H:%M:%S%z" - "%Y-%m-%dT%H:%M:%S.%fZ" - "%Y-%m-%dT%H:%M:%S.%f%z" - "%Y-%m-%d %H:%M:%S.%f+00:00" - "%s" - "%ms" - - "%s_as_float" + - "%s_as_float" + - "%a, %d %b %Y %H:%M:%S %z" + - "%Y-%m-%dT%H:%M:%S.%_msZ" + - "%Y-%m-%dT%H:%M:%S.%_ms%z"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
(5 hunks)
⏰ 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). (9)
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-intercom
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-shopify
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: SDM Docker Image Build
Once https://github.com/airbytehq/airbyte-platform-internal/pull/17519 is merged, the Connector Builder will automatically render a list of values when
suggestions
is set on astring
orarray<string>
field.This PR adds common values to the
suggestions
properties of various DatetimeBasedCursor fields, as these tend to be confusing for users to configure and having suggestions makes this easier.Summary by CodeRabbit