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

Support multiple datetime format for cursor field #28782

Closed
maxi297 opened this issue Jul 27, 2023 · 1 comment
Closed

Support multiple datetime format for cursor field #28782

maxi297 opened this issue Jul 27, 2023 · 1 comment
Assignees
Labels

Comments

@maxi297
Copy link
Contributor

maxi297 commented Jul 27, 2023

What area the feature impact?

Connectors

Revelant Information

Business Central API seems to return different datetime format for the cursor field: %Y-%m-%dT%H:%M:%S.%fZ and %Y-%m-%dT%H:%M:%SZ. This means that configuring an incremental sync is not possible for this API.

Proposed solution

Allow for multiple datetime formats. If multiple datetime_format are provided, start_datetime and end_datetime needs to define a datetime_format

  incremental_sync:
    type: "DatetimeBasedCursor"
    start_datetime:
      datetime: {{ config['start_date'] }}
      datetime_format: "%Y-%m-%d %H:%M:%S"
    end_datetime:
      datetime: {{ config['end_date'] }}
      datetime_format: "%Y-%m-%d %H:%M:%S"
    datetime_format: "%Y-%m-%dT%H:%M:%SZ"
    cursor_datetime_format: 
      - "%Y-%m-%dT%H:%M:%SZ"
      - "%Y-%m-%dT%H:%M:%S.%fZ"
    cursor_field: "publishedAt"

Dependency

If we allow datetime_format to be a list, it doesn't align with #27153. Possible solutions:

  • do this issue before so that cursor_datetime_format can be a list and not datetime_format
  • have outgoing_datetime_format instead of cursor_datetime_format and keep datetime_format for the cursor
@maxi297
Copy link
Contributor Author

maxi297 commented Aug 1, 2023

Grooming notes:

  • cursor_datetime_format should be in order so that if the connector dev knows that one is more frequent than the other, he can defined it first and this will be more performant
  • to avoid breaking changes, if cursor_datetime_format is not defined, use datetime_format
  • cursor_datetime_format is a List[str]

maxi297 added a commit that referenced this issue Aug 1, 2023
* [ISSUE #28782] support multiple cursor field datetime formats

* Making sure we use the proper format for creating slices

* Code review
@maxi297 maxi297 self-assigned this Aug 1, 2023
@maxi297 maxi297 closed this as completed Aug 2, 2023
bnchrch pushed a commit that referenced this issue Aug 3, 2023
* [ISSUE #28782] support multiple cursor field datetime formats

* Making sure we use the proper format for creating slices

* Code review
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

1 participant