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

Jamakase/select default sync modes #10320

Merged
merged 3 commits into from
Mar 5, 2022
Merged

Jamakase/select default sync modes #10320

merged 3 commits into from
Mar 5, 2022

Conversation

jamakase
Copy link
Contributor

@jamakase jamakase commented Feb 14, 2022 •
edited

What

Closes #9625

How

In create mode, we will try to set next values for sync mode by default:

If there is a source-defined cursor on a stream, the best sync modes, in order, are:

append dedupe
append
If there is no cursor defined:

full refresh overwrite

@github-actions github-actions bot added area/frontend area/platform issues related to the platform labels Feb 14, 2022
@jamakase jamakase temporarily deployed to more-secrets February 14, 2022 17:02 Inactive
@jrhizor
Copy link
Contributor

jrhizor commented Feb 14, 2022

@sherifnada Would you be able to run through this or have someone on connectors run through this to see if it gives the desired experience?

@sherifnada
Copy link
Contributor

@jrhizor is the PR ready in its current state to test?

@jamakase jamakase force-pushed the jamakase/select-default-sync-modes branch from 9c7fc36 to 9173867 Compare February 21, 2022 13:08
@jamakase jamakase marked this pull request as ready for review February 21, 2022 13:08
@jamakase jamakase temporarily deployed to more-secrets February 21, 2022 13:10 Inactive
@jamakase jamakase temporarily deployed to more-secrets February 21, 2022 13:10 Inactive
@jamakase jamakase self-assigned this Feb 22, 2022
@timroes
Copy link
Collaborator

timroes commented Feb 23, 2022

Could we rebase that on master due to the dependency upgrade for the review please.

@jamakase jamakase force-pushed the jamakase/select-default-sync-modes branch from 9173867 to f435cdd Compare February 25, 2022 11:53
@jrhizor
Copy link
Contributor

jrhizor commented Feb 25, 2022

Please let @sherifnada know when this is ready to test.

@jamakase jamakase requested a review from a team as a code owner March 1, 2022 16:25
@jamakase
Copy link
Contributor Author

jamakase commented Mar 1, 2022

@sherifnada it is ready to be checked

@jamakase jamakase temporarily deployed to more-secrets March 1, 2022 16:27 Inactive
@jamakase jamakase temporarily deployed to more-secrets March 1, 2022 16:27 Inactive
jamakase
jamakase commented Mar 1, 2022
View reviewed changes
airbyte-webapp/src/setupTests.tsx Outdated
@@ -4,3 +4,13 @@
// learn more: https://github.com/testing-library/jest-dom
import "@testing-library/jest-dom/extend-expect";
import "@testing-library/jest-dom";
import React from "react";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timroes please note - I moved this fix to be global instead of ServiceForm specific

@jamakase jamakase temporarily deployed to more-secrets March 3, 2022 07:42 Inactive
@jamakase jamakase temporarily deployed to more-secrets March 3, 2022 07:45 Inactive
@jamakase jamakase temporarily deployed to more-secrets March 3, 2022 12:05 Inactive
@jamakase jamakase temporarily deployed to more-secrets March 3, 2022 12:05 Inactive
@sherifnada
Copy link
Contributor

Demoe'd it and it looks great! Count me a happy user

timroes
timroes approved these changes Mar 4, 2022
View reviewed changes
Copy link
Collaborator

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Code LGTM. Haven't tested locally.

@jamakase jamakase merged commit ff4762c into master Mar 5, 2022
@jamakase jamakase deleted the jamakase/select-default-sync-modes branch March 5, 2022 09:09
@octavia-squidington-iii octavia-squidington-iii mentioned this pull request Mar 8, 2022
Merged
benmoriceau added a commit that referenced this pull request Mar 11, 2022
@benmoriceau
Revert "Jamakase/select default sync modes (#10320)"
0026176 
This reverts commit ff4762c.
@benmoriceau benmoriceau mentioned this pull request Mar 11, 2022
Merged
benmoriceau added a commit that referenced this pull request Mar 12, 2022
@benmoriceau
Revert "Jamakase/select default sync modes (#10320)" (#11080)
45c970e 
This reverts commit ff4762c.
terencecho pushed a commit that referenced this pull request Mar 14, 2022
@benmoriceau @terencecho
Revert "Jamakase/select default sync modes (#10320)" (#11080)
1108910 
This reverts commit ff4762c.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timroes timroes timroes approved these changes

@jrhizor jrhizor Awaiting requested review from jrhizor

@juliachvyrova juliachvyrova Awaiting requested review from juliachvyrova

Assignees

@jamakase jamakase

Labels
area/platform issues related to the platform
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Choose optimal sync modes by default on UI
4 participants
@jamakase @jrhizor @sherifnada @timroes