Common Jsons: add flag to apply flatten to arrays #17273

adam-bloom · 2022-09-27T18:31:50Z

What

Continues #13464
Adds unit tests and resolves recursive logic error.

How

Describe the solution

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

marcosmarxm · 2022-09-30T16:11:27Z

Thanks for finishing this contribution @adam-bloom, @tuliren can you help reviewing it?

marcosmarxm · 2022-10-14T14:08:21Z

/test connector=connectors/destination-redshift

🕑 connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/3250573522
✅ connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/3250573522
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 14      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    14      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     166      8    95%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_config/transform.py                         191     49    74%
normalization/transform_catalog/utils.py                             51     14    73%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 589    394    33%
-------------------------------------------------------------------------------------
TOTAL                                                              1437    624    57%

Build Passed

Test summary info:

All Passed

marcosmarxm · 2022-11-22T20:23:29Z

Hello 👋, first thank you for this amazing contribution.

We really appreciate the effort you've made to improve the project.
We ask you patience for the code review. Last month our team was focused on Hacktoberfest event and that probably left some PR without the proper feedback. And this week, due to the Thanksgiving US Holiday, most our team is out of office with their families. Another important piece of information why code won't be merge this week is: as a safety measure the core team has decided to freeze merging code to main branch to keep the release stable. Next week we'll return to you with the proper code review and update the status of your contribution.

If you have any questions feel free to send me a message in Slack!
Thanks!

marcosmarxm · 2022-12-08T19:31:21Z

/test connector=connectors/destination-redshift

🕑 connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/3651481515
❌ connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/3651481515
🐛 https://gradle.com/s/3sak2p5og6ah2

Build Failed

Test summary info:

Could not find result summary

marcosmarxm · 2022-12-08T23:38:26Z

/test connector=connectors/destination-redshift

🕑 connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/3653048760
❌ connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/3653048760
🐛 https://gradle.com/s/h4ednqlyt24sa

Build Failed

Test summary info:

Could not find result summary

marcosmarxm · 2022-12-20T19:50:34Z

Hello 👋:skin-tone-2: and thank you for your contribution!

Airbyte has instituted a code freeze between 19 and 30 December, to make sure there are no disruptions during the holidays.
Because of this, reviewing and merging your contribution may take longer than usual.
We apologize for the delay, but we want everyone to have a quiet and happy holiday.

If you have any questions or need further clarification, please don't hesitate to ping via Slack.

...persistence/job-persistence/src/main/java/io/airbyte/persistence/job/tracker/JobTracker.java

...main/java/io/airbyte/integrations/destination/redshift/operations/RedshiftSqlOperations.java

grishick

LGTM, just remove the change from JobTracker (let it use the old method signature)

…son-function

grishick · 2023-01-03T21:24:44Z

/test connector=connectors/destination-redshift

🕑 connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/3833099848
✅ connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/3833099848
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 14      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    14      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     166      8    95%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_config/transform.py                         189     48    75%
normalization/transform_catalog/utils.py                             51     14    73%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 595    400    33%
-------------------------------------------------------------------------------------
TOTAL                                                              1441    629    56%

Build Passed

Test summary info:

All Passed

grishick · 2023-01-03T22:56:18Z

Running CI checks here: #20993 and will publish and merge from this PR

grishick · 2023-01-05T00:38:44Z

CI checks passed in this PR: #20993

grishick · 2023-01-05T21:00:58Z

/publish connector=connectors/destination-redshift

🕑 Publishing the following connectors:
connectors/destination-redshift
https://github.com/airbytehq/airbyte/actions/runs/3850331793

Connector	Did it publish?	Were definitions generated?
connectors/destination-redshift	❌	❌

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

grishick · 2023-01-05T23:32:49Z

Thank you @adam-bloom , your changes were merged in this PR: #20993
New version of destination-redshift has been published in this PR: #21083

adam-bloom requested a review from a team as a code owner September 27, 2022 18:31

github-actions bot added the area/connectors Connector related issues label Sep 27, 2022

octavia-squidington-iii added bounty community connectors/destination/redshift labels Sep 27, 2022

adam-bloom force-pushed the correct-flatten-json-function branch from c0047b6 to 768df52 Compare September 27, 2022 18:37

github-actions bot added area/platform issues related to the platform area/documentation Improvements or additions to documentation labels Sep 27, 2022

sajarin added area/databases internal and removed bounty labels Sep 28, 2022

marcosmarxm requested a review from tuliren September 30, 2022 16:11

marcosmarxm mentioned this pull request Oct 14, 2022

Destination Redshift: CONCAT() result too long for type varchar(65535) #16412

Closed

marcosmarxm mentioned this pull request Oct 14, 2022

(running CI acceptance test changes in platform) Community PR 17273 #18001

Closed

37 tasks

marcosmarxm force-pushed the correct-flatten-json-function branch from 0d305a8 to 491f5ff Compare October 14, 2022 17:47

octavia-squidington-iv added connectors/destination/bitdotio connectors/source/shopify connectors/source/jdbc labels Oct 14, 2022

marcosmarxm and others added 7 commits October 19, 2022 09:52

add flag to apply flatten to arrays

25d955c

run format change docstirng

56d79f4

fix recursion

a2838a2

add additional unit test cases for array flattening

cf38f73

add backward compatibility function

52d05c8

bump dest-redshift version and add changelog

e81a5f1

format

682c55e

adam-bloom force-pushed the correct-flatten-json-function branch from 491f5ff to 682c55e Compare October 19, 2022 15:53

davinchia and others added 2 commits December 22, 2022 16:32

Merge branch 'master' into correct-flatten-json-function

319c636

Merge branch 'master' into correct-flatten-json-function

55b9bd3

grishick reviewed Dec 28, 2022

View reviewed changes

...persistence/job-persistence/src/main/java/io/airbyte/persistence/job/tracker/JobTracker.java Outdated Show resolved Hide resolved

grishick reviewed Dec 28, 2022

View reviewed changes

...main/java/io/airbyte/integrations/destination/redshift/operations/RedshiftSqlOperations.java Outdated Show resolved Hide resolved

grishick suggested changes Dec 28, 2022

View reviewed changes

use original method signature

0e55dd5

adam-bloom requested review from grishick and removed request for tuliren December 29, 2022 14:38

octavia-squidington-iv removed the area/platform issues related to the platform label Dec 29, 2022

grishick approved these changes Dec 29, 2022

View reviewed changes

adam-bloom and others added 2 commits January 3, 2023 09:35

Merge remote-tracking branch 'upstream/master' into correct-flatten-j…

9f6f01e

…son-function

Merge branch 'master' into correct-flatten-json-function

8f4da88

grishick mentioned this pull request Jan 3, 2023

Common Jsons: add flag to apply flatten to arrays #20993

Merged

grishick temporarily deployed to more-secrets January 3, 2023 21:35 — with GitHub Actions Inactive

Merge branch 'master' into correct-flatten-json-function

3a7e592

grishick added 3 commits January 3, 2023 15:10

Merge branch 'master' into correct-flatten-json-function

c440c65

Merge branch 'master' into correct-flatten-json-function

c71b1b0

Merge branch 'master' into correct-flatten-json-function

4d5db1f

Merge branch 'master' into correct-flatten-json-function

0248828

octavia-squidington-iv removed area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Jan 5, 2023

grishick closed this Jan 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Common Jsons: add flag to apply flatten to arrays #17273

Common Jsons: add flag to apply flatten to arrays #17273

adam-bloom commented Sep 27, 2022

marcosmarxm commented Sep 30, 2022

marcosmarxm commented Oct 14, 2022 •

edited by github-actions bot

Loading

marcosmarxm commented Nov 22, 2022

marcosmarxm commented Dec 8, 2022 •

edited by github-actions bot

Loading

marcosmarxm commented Dec 8, 2022 •

edited by github-actions bot

Loading

marcosmarxm commented Dec 20, 2022

grishick left a comment

grishick commented Jan 3, 2023 •

edited by github-actions bot

Loading

grishick commented Jan 3, 2023 •

edited

Loading

grishick commented Jan 5, 2023

grishick commented Jan 5, 2023 •

edited by github-actions bot

Loading

grishick commented Jan 5, 2023

Common Jsons: add flag to apply flatten to arrays #17273

Common Jsons: add flag to apply flatten to arrays #17273

Conversation

adam-bloom commented Sep 27, 2022

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

marcosmarxm commented Sep 30, 2022

marcosmarxm commented Oct 14, 2022 • edited by github-actions bot Loading

Build Passed

marcosmarxm commented Nov 22, 2022

marcosmarxm commented Dec 8, 2022 • edited by github-actions bot Loading

Build Failed

marcosmarxm commented Dec 8, 2022 • edited by github-actions bot Loading

Build Failed

marcosmarxm commented Dec 20, 2022

grishick left a comment

Choose a reason for hiding this comment

grishick commented Jan 3, 2023 • edited by github-actions bot Loading

Build Passed

grishick commented Jan 3, 2023 • edited Loading

grishick commented Jan 5, 2023

grishick commented Jan 5, 2023 • edited by github-actions bot Loading

grishick commented Jan 5, 2023

marcosmarxm commented Oct 14, 2022 •

edited by github-actions bot

Loading

marcosmarxm commented Dec 8, 2022 •

edited by github-actions bot

Loading

marcosmarxm commented Dec 8, 2022 •

edited by github-actions bot

Loading

grishick commented Jan 3, 2023 •

edited by github-actions bot

Loading

grishick commented Jan 3, 2023 •

edited

Loading

grishick commented Jan 5, 2023 •

edited by github-actions bot

Loading