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.

Sign up for GitHub

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

Jump to bottom

[ISSUE #28782] support multiple cursor field datetime formats #28936

Merged
merged 3 commits into from
Aug 1, 2023
Merged

[ISSUE #28782] support multiple cursor field datetime formats #28936

merged 3 commits into from
Aug 1, 2023

Conversation

maxi297
Copy link
Contributor

@maxi297 maxi297 commented Aug 1, 2023

What

Addresses #28782 and allow to fix issues in prod for Amplitude

@maxi297 maxi297 requested a review from a team as a code owner August 1, 2023 19:37
@octavia-squidington-iii octavia-squidington-iii added the CDK Connector Development Kit label Aug 1, 2023
@maxi297 maxi297 requested a review from girarda August 1, 2023 19:40
@maxi297
Copy link
Contributor Author

maxi297 commented Aug 1, 2023

There is the mypy situation going on but I don't know if it's worth fixing this while having an issue in production

clnoll
clnoll approved these changes Aug 1, 2023
View reviewed changes
Copy link
Contributor

@clnoll clnoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few minor comments.

airbyte-cdk/python/airbyte_cdk/sources/declarative/incremental/datetime_based_cursor.py Show resolved Hide resolved
airbyte-cdk/python/airbyte_cdk/sources/declarative/incremental/datetime_based_cursor.py Show resolved Hide resolved
airbyte-cdk/python/unit_tests/sources/declarative/incremental/test_datetime_based_cursor.py Show resolved Hide resolved
brianjlai
brianjlai approved these changes Aug 1, 2023
View reviewed changes
Copy link
Contributor

@brianjlai brianjlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only non-blocking comments. lgtm

airbyte-cdk/python/airbyte_cdk/sources/declarative/declarative_component_schema.yaml Outdated Show resolved Hide resolved
airbyte-cdk/python/airbyte_cdk/sources/declarative/incremental/datetime_based_cursor.py Outdated Show resolved Hide resolved
@maxi297 maxi297 merged commit e158bec into master Aug 1, 2023
@maxi297 maxi297 deleted the issue-28782/add-many-cursor-datetime-formats branch August 1, 2023 21:59
@lmossman lmossman mentioned this pull request Aug 3, 2023
Merged
bnchrch pushed a commit that referenced this pull request Aug 3, 2023
@maxi297 @bnchrch
[ISSUE #28782] support multiple cursor field datetime formats (#28936)
44afab5 
* [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
Reviewers

@brianjlai brianjlai brianjlai approved these changes

@clnoll clnoll clnoll approved these changes

@girarda girarda Awaiting requested review from girarda

Assignees
No one assigned
Labels
CDK Connector Development Kit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@maxi297 @clnoll @brianjlai @octavia-squidington-iii