db-sources:sort primary key based on key-seq #29346

subodh1810 · 2023-08-10T20:44:01Z

Issue : #28699

github-actions · 2023-08-10T20:44:22Z

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

PR name follows PR naming conventions
Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
Secrets in the connector's spec are annotated with airbyte_secret
All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

Check for hidden checklists in your PR description
Toggle the github label checklist-action-run on/off to re-run the checklist CI.

subodh1810 · 2023-08-10T20:44:32Z

/legacy-test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/5825924122
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/5825924122
No Python unittests run

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:106: The previous and actual specifications are identical.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:631: The previous and actual discovered catalogs are identical.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:728: This tests currently leads to too much failures. We need to fix the connectors at scale first.
================== 70 passed, 7 skipped in 106.81s (0:01:46) ===================

subodh1810 · 2023-08-10T20:44:41Z

/legacy-test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/5825924964
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/5825924964
No Python unittests run

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestConnection.test_check: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestDiscovery.test_discover: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestBasicRead.test_read: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestFullRefresh.test_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:106: The previous and actual specifications are identical.
================= 23 passed, 6 skipped, 31 warnings in 30.94s ==================

subodh1810 · 2023-08-10T20:44:47Z

/legacy-test connector=connectors/source-mssql

🕑 connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/5825925673
✅ connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/5825925673
No Python unittests run

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestConnection.test_check: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestDiscovery.test_discover: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestBasicRead.test_read: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestFullRefresh.test_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:106: The previous and actual specifications are identical.
================= 23 passed, 6 skipped, 31 warnings in 30.90s ==================

prateekmukhedkar · 2023-08-10T20:59:57Z

...ectors/source-jdbc/src/main/java/io/airbyte/integrations/source/jdbc/AbstractJdbcSource.java

@@ -146,13 +148,14 @@ protected List<CheckedConsumer<JdbcDatabase, Exception>> getCheckOperations(fina
   *
   * @return a map by StreamName to associated list of primary keys
   */
-  private static Map<String, List<String>> aggregatePrimateKeys(final List<SimpleImmutableEntry<String, String>> entries) {
+  @VisibleForTesting
+  public static Map<String, List<String>> aggregatePrimateKeys(final List<PrimaryKeyAttributesFromDb> entries) {


interesting, this change is in the base classes - do we need to run Postgres tests to ensure no side effect? cc @akashkulk

we should, but I double there will be too much of a side effect here (just changing the order of keys)

Already ran! I dont expect anything to change.

octavia-squidington-iii · 2023-08-10T21:07:05Z

source-mysql test report (commit `ed48150187`) - ❌

⏲️ Total pipeline duration: 21mn09s

Step	Result
Validate airbyte-integrations/connectors/source-mysql/metadata.yaml	✅
Connector version semver check	✅
QA checks	✅
Build connector tar	✅
Build source-mysql docker image for platform linux/x86_64	✅
./gradlew :airbyte-integrations:connectors:source-mysql:integrationTest	❌
Acceptance tests	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mysql test

akashkulk

Approving - we should just run tests for relevant connectors (mssql/mysql/postgres) just to ensure they don't check for order of keys in the catalog or something

akashkulk · 2023-08-15T18:33:43Z

/legacy-test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/5870488516
❌ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/5870488516
🐛 https://gradle.com/s/oq6mu3cklkvgw

Build Failed

Test summary info:

Could not find result summary

akashkulk · 2023-08-15T18:36:11Z

/legacy-test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/5870510529
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/5870510529
No Python unittests run

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../app/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [2] ../app/connector_acceptance_test/tests/test_core.py:103: The previous and actual specifications are identical.
SKIPPED [2] ../app/connector_acceptance_test/tests/test_core.py:623: The previous and actual discovered catalogs are identical.
SKIPPED [2] ../app/connector_acceptance_test/tests/test_core.py:720: This tests currently leads to too much failures. We need to fix the connectors at scale first.
============ 70 passed, 7 skipped, 2 warnings in 705.67s (0:11:45) =============

octavia-squidington-iii · 2023-08-15T18:55:15Z

source-mysql test report (commit `1abf96e853`) - ❌

⏲️ Total pipeline duration: 20mn32s

Step	Result
Validate airbyte-integrations/connectors/source-mysql/metadata.yaml	✅
Connector version semver check	✅
QA checks	✅
Build connector tar	✅
Build source-mysql docker image for platform linux/x86_64	✅
./gradlew :airbyte-integrations:connectors:source-mysql:integrationTest	❌
Acceptance tests	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mysql test

akashkulk · 2023-08-15T21:19:24Z

/legacy-test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/5871861787
❌ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/5871861787
🐛 https://gradle.com/s/25uvwns266b4o

Build Failed

Test summary info:

Could not find result summary

akashkulk · 2023-08-15T22:46:29Z

/approve-and-merge reason="mysql tests passing locally, some CI flakiness"

octavia-approvington · 2023-08-15T22:47:09Z

Enough! Let's do this

Co-authored-by: Akash Kulkarni <113392464+akashkulk@users.noreply.github.com>

db-sources:sort primary key based on key-seq

ed48150

subodh1810 self-assigned this Aug 10, 2023

subodh1810 requested a review from a team as a code owner August 10, 2023 20:44

octavia-squidington-iii added area/connectors Connector related issues connectors/source/mysql connectors/source/jdbc labels Aug 10, 2023

subodh1810 requested a review from akashkulk August 10, 2023 20:45

prateekmukhedkar reviewed Aug 10, 2023

View reviewed changes

akashkulk approved these changes Aug 10, 2023

View reviewed changes

prateekmukhedkar self-requested a review August 10, 2023 21:24

prateekmukhedkar approved these changes Aug 10, 2023

View reviewed changes

Merge branch 'master' into sort-pkey-key-seq-basis

1abf96e

octavia-approvington approved these changes Aug 15, 2023

View reviewed changes

octavia-approvington merged commit cc85da9 into master Aug 15, 2023

octavia-approvington deleted the sort-pkey-key-seq-basis branch August 15, 2023 22:47

akashkulk mentioned this pull request Aug 18, 2023

source-mysql : Chunking logic sync bugs in case of composite primary key #29574

Closed

harrytou pushed a commit to KYVENetwork/airbyte that referenced this pull request Sep 1, 2023

db-sources:sort primary key based on key-seq (airbytehq#29346)

3a33a34

Co-authored-by: Akash Kulkarni <113392464+akashkulk@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

db-sources:sort primary key based on key-seq #29346

db-sources:sort primary key based on key-seq #29346

subodh1810 commented Aug 10, 2023

github-actions bot commented Aug 10, 2023

subodh1810 commented Aug 10, 2023 •

edited by github-actions bot

Loading

subodh1810 commented Aug 10, 2023 •

edited by github-actions bot

Loading

subodh1810 commented Aug 10, 2023 •

edited by github-actions bot

Loading

prateekmukhedkar Aug 10, 2023 •

edited

Loading

akashkulk Aug 10, 2023

subodh1810 Aug 10, 2023

octavia-squidington-iii commented Aug 10, 2023

akashkulk left a comment

akashkulk commented Aug 15, 2023 •

edited by github-actions bot

Loading

akashkulk commented Aug 15, 2023 •

edited by github-actions bot

Loading

octavia-squidington-iii commented Aug 15, 2023

akashkulk commented Aug 15, 2023 •

edited by github-actions bot

Loading

akashkulk commented Aug 15, 2023

octavia-approvington commented Aug 15, 2023

db-sources:sort primary key based on key-seq #29346

db-sources:sort primary key based on key-seq #29346

Conversation

subodh1810 commented Aug 10, 2023

github-actions bot commented Aug 10, 2023

Before Merging a Connector Pull Request

subodh1810 commented Aug 10, 2023 • edited by github-actions bot Loading

Build Passed

subodh1810 commented Aug 10, 2023 • edited by github-actions bot Loading

Build Passed

subodh1810 commented Aug 10, 2023 • edited by github-actions bot Loading

Build Passed

prateekmukhedkar Aug 10, 2023 • edited Loading

Choose a reason for hiding this comment

akashkulk Aug 10, 2023

Choose a reason for hiding this comment

subodh1810 Aug 10, 2023

Choose a reason for hiding this comment

octavia-squidington-iii commented Aug 10, 2023

source-mysql test report (commit ed48150187) - ❌

akashkulk left a comment

Choose a reason for hiding this comment

akashkulk commented Aug 15, 2023 • edited by github-actions bot Loading

Build Failed

akashkulk commented Aug 15, 2023 • edited by github-actions bot Loading

Build Passed

octavia-squidington-iii commented Aug 15, 2023

source-mysql test report (commit 1abf96e853) - ❌

akashkulk commented Aug 15, 2023 • edited by github-actions bot Loading

Build Failed

akashkulk commented Aug 15, 2023

octavia-approvington commented Aug 15, 2023

subodh1810 commented Aug 10, 2023 •

edited by github-actions bot

Loading

subodh1810 commented Aug 10, 2023 •

edited by github-actions bot

Loading

subodh1810 commented Aug 10, 2023 •

edited by github-actions bot

Loading

prateekmukhedkar Aug 10, 2023 •

edited

Loading

source-mysql test report (commit `ed48150187`) - ❌

akashkulk commented Aug 15, 2023 •

edited by github-actions bot

Loading

akashkulk commented Aug 15, 2023 •

edited by github-actions bot

Loading

source-mysql test report (commit `1abf96e853`) - ❌

akashkulk commented Aug 15, 2023 •

edited by github-actions bot

Loading