Skip to content
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

Make DatetimeBasedCursor.end_datetime optional #26568

Closed
3 tasks
maxi297 opened this issue May 25, 2023 · 0 comments
Closed
3 tasks

Make DatetimeBasedCursor.end_datetime optional #26568

maxi297 opened this issue May 25, 2023 · 0 comments
Assignees
Labels

Comments

@maxi297
Copy link
Contributor

maxi297 commented May 25, 2023

What area the feature impact?

Connectors

Revelant Information

Tell us about the problem you're trying to solve

Today, users wanting to configure an incremental sync using DatetimeBasedCursor need to provide a end_datetime. However, the vast majority of users user a combination of today_utc() when the granularity is longer than a day, now_utc() when the granularity is smaller than a day or config["end_datetime"].

Describe the solution you’d like

The default behavior should be to use now_utc() as the value of the end_datetime. Therefore, following

    incremental_sync:
      type: "DatetimeBasedCursor"
      start_datetime:
        datetime: "{{ config['start_date'] }}"
        datetime_format: "%Y-%m"
      step: "P1M"
      datetime_format: "%Y-%m-%dT%H:%M:%S%z"
      cursor_granularity: "PT1S"
      cursor_field: "pub_date"

... is the equivalent of:

    incremental_sync:
      type: "DatetimeBasedCursor"
      start_datetime:
        datetime: "{{ config['start_date'] }}"
        datetime_format: "%Y-%m"
      end_datetime:
        datetime: "{{ now_utc() }}"
        datetime_format: <format for now_utc()>
      step: "P1M"
      datetime_format: "%Y-%m-%dT%H:%M:%S%z"
      cursor_granularity: "PT1S"
      cursor_field: "pub_date"

Note that the end_datetime.datetime_format is using the same as start_datetime.datetime_format. If this is not the case, the user will have the explicitly define end_datetime.datetime_format.
If the connector developer wants to allow the end-user to use config, he will have t

Note that by default, we don't allow use `"{{ config['end_datetime'] or now_utc() }}". This is to avoid unexpected behavior where the connector developer defines a variable that clash with this one.

Acceptance Criteria

  • Implement the proposed solution in the CDK
  • Update the Connector Builder (UI is fine but it's about state management)
  • Update the documentation
@maxi297 maxi297 self-assigned this Jun 5, 2023
maxi297 added a commit that referenced this issue Jun 8, 2023
maxi297 added a commit that referenced this issue Jun 8, 2023
* [ISSUE #26568] make DatetimeBasedCursor.end_datetime optional

* [ISSUE #26568] ensure model_to_component_factory create end_datetime properly

* [ISSUE #26568] fix typing

* [ISSUE #26568] improve end_datetime documentation
@maxi297 maxi297 closed this as completed Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants