S3, GCS Destinations : Fix interface duplication #9577

DoNotPanicUA · 2022-01-18T15:22:54Z

What

#8894
Remove logical duplication from the writer interfaces.
Is blocked by #6063

How

Create new generic interfaces at the bottom (S3 destination is the lowest level right now for writers). Remove duplication interfaces from the GCS destination.

Recommended reading order

DestinationFileWriter.java
DestinationWriter.java

…-writer-interf

alexandr-shegeda · 2022-01-21T12:02:29Z

...ke/src/test/java/io/airbyte/integrations/destination/snowflake/SnowflakeDestinationTest.java

@@ -73,7 +73,7 @@ public void useInsertStrategyTest() {
    assertFalse(SnowflakeDestination.isS3Copy(stubConfig));
  }

-  @Test
+  //@Test


are you going to uncomment this test?

please check do we need this test?

…-writer-interf

mkhokh-33 · 2022-01-21T13:18:40Z

...ke/src/test/java/io/airbyte/integrations/destination/snowflake/SnowflakeDestinationTest.java

@@ -73,7 +73,7 @@ public void useInsertStrategyTest() {
    assertFalse(SnowflakeDestination.isS3Copy(stubConfig));
  }

-  @Test
+  //@Test


please check do we need this test?

mkhokh-33 · 2022-01-21T13:20:42Z

...destination-s3/src/main/java/io/airbyte/integrations/destination/s3/writer/BaseS3Writer.java

@@ -34,7 +34,7 @@
 * <li>Log and close the write.</li>
 * </ul>
 */
-public abstract class BaseS3Writer implements S3Writer {
+public abstract class BaseS3Writer implements DestinationFileWriter {

  private static final Logger LOGGER = LoggerFactory.getLogger(BaseS3Writer.class);
  public static final String DEFAULT_SUFFIX = "_0";


I guess we could change it to private as constant is used only inside this class :)

alexandertsukanov

No critical remarks from my side.

…-writer-interf

DoNotPanicUA · 2022-01-24T13:22:43Z

/test connector=connectors/destination-s3

🕑 connectors/destination-s3 https://github.com/airbytehq/airbyte/actions/runs/1740065860
✅ connectors/destination-s3 https://github.com/airbytehq/airbyte/actions/runs/1740065860
No Python unittests run

DoNotPanicUA · 2022-01-24T13:22:51Z

/test connector=connectors/destination-gcs

🕑 connectors/destination-gcs https://github.com/airbytehq/airbyte/actions/runs/1740066285
❌ connectors/destination-gcs https://github.com/airbytehq/airbyte/actions/runs/1740066285
🐛

This fail is not result of the PR. This error is in the mater last 5 days

DoNotPanicUA · 2022-01-24T13:23:00Z

/test connector=connectors/destination-snowflake

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

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                              Stmts   Miss  Cover
	 -------------------------------------------------------------------------------------
	 main_dev_transform_catalog.py                                         3      3     0%
	 main_dev_transform_config.py                                          3      3     0%
	 normalization/__init__.py                                             4      0   100%
	 normalization/destination_type.py                                    13      0   100%
	 normalization/transform_catalog/__init__.py                           2      0   100%
	 normalization/transform_catalog/catalog_processor.py                143     77    46%
	 normalization/transform_catalog/destination_name_transformer.py     155      8    95%
	 normalization/transform_catalog/reserved_keywords.py                 13      0   100%
	 normalization/transform_catalog/stream_processor.py                 520    333    36%
	 normalization/transform_catalog/table_name_registry.py              174     34    80%
	 normalization/transform_catalog/transform.py                         45     26    42%
	 normalization/transform_catalog/utils.py                             33      7    79%
	 normalization/transform_config/__init__.py                            2      0   100%
	 normalization/transform_config/transform.py                         148     34    77%
	 -------------------------------------------------------------------------------------
	 TOTAL                                                              1258    525    58%

DoNotPanicUA · 2022-01-24T13:23:08Z

/test connector=connectors/destination-bigquery

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

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                              Stmts   Miss  Cover
	 -------------------------------------------------------------------------------------
	 main_dev_transform_catalog.py                                         3      3     0%
	 main_dev_transform_config.py                                          3      3     0%
	 normalization/__init__.py                                             4      0   100%
	 normalization/destination_type.py                                    13      0   100%
	 normalization/transform_catalog/__init__.py                           2      0   100%
	 normalization/transform_catalog/catalog_processor.py                143     77    46%
	 normalization/transform_catalog/destination_name_transformer.py     155      8    95%
	 normalization/transform_catalog/reserved_keywords.py                 13      0   100%
	 normalization/transform_catalog/stream_processor.py                 520    333    36%
	 normalization/transform_catalog/table_name_registry.py              174     34    80%
	 normalization/transform_catalog/transform.py                         45     26    42%
	 normalization/transform_catalog/utils.py                             33      7    79%
	 normalization/transform_config/__init__.py                            2      0   100%
	 normalization/transform_config/transform.py                         148     34    77%
	 -------------------------------------------------------------------------------------
	 TOTAL                                                              1258    525    58%

DoNotPanicUA · 2022-01-24T13:23:19Z

/test connector=connectors/destination-bigquery-denormalized

🕑 connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/1740068482
✅ connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/1740068482
Python tests coverage:

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                              Stmts   Miss  Cover
	 -------------------------------------------------------------------------------------
	 main_dev_transform_catalog.py                                         3      3     0%
	 main_dev_transform_config.py                                          3      3     0%
	 normalization/__init__.py                                             4      0   100%
	 normalization/destination_type.py                                    13      0   100%
	 normalization/transform_catalog/__init__.py                           2      0   100%
	 normalization/transform_catalog/catalog_processor.py                143     77    46%
	 normalization/transform_catalog/destination_name_transformer.py     155      8    95%
	 normalization/transform_catalog/reserved_keywords.py                 13      0   100%
	 normalization/transform_catalog/stream_processor.py                 520    333    36%
	 normalization/transform_catalog/table_name_registry.py              174     34    80%
	 normalization/transform_catalog/transform.py                         45     26    42%
	 normalization/transform_catalog/utils.py                             33      7    79%
	 normalization/transform_config/__init__.py                            2      0   100%
	 normalization/transform_config/transform.py                         148     34    77%
	 -------------------------------------------------------------------------------------
	 TOTAL                                                              1258    525    58%

DoNotPanicUA · 2022-01-24T14:01:02Z

/test connector=connectors/destination-s3

🕑 connectors/destination-s3 https://github.com/airbytehq/airbyte/actions/runs/1740234649
✅ connectors/destination-s3 https://github.com/airbytehq/airbyte/actions/runs/1740234649
No Python unittests run

DoNotPanicUA · 2022-01-24T14:01:16Z

/test connector=connectors/destination-gcs

🕑 connectors/destination-gcs https://github.com/airbytehq/airbyte/actions/runs/1740235920
❌ connectors/destination-gcs https://github.com/airbytehq/airbyte/actions/runs/1740235920
🐛

DoNotPanicUA · 2022-01-24T14:01:26Z

/test connector=connectors/destination-bigquery

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

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                              Stmts   Miss  Cover
	 -------------------------------------------------------------------------------------
	 main_dev_transform_catalog.py                                         3      3     0%
	 main_dev_transform_config.py                                          3      3     0%
	 normalization/__init__.py                                             4      0   100%
	 normalization/destination_type.py                                    13      0   100%
	 normalization/transform_catalog/__init__.py                           2      0   100%
	 normalization/transform_catalog/catalog_processor.py                143     77    46%
	 normalization/transform_catalog/destination_name_transformer.py     155      8    95%
	 normalization/transform_catalog/reserved_keywords.py                 13      0   100%
	 normalization/transform_catalog/stream_processor.py                 520    333    36%
	 normalization/transform_catalog/table_name_registry.py              174     34    80%
	 normalization/transform_catalog/transform.py                         45     26    42%
	 normalization/transform_catalog/utils.py                             33      7    79%
	 normalization/transform_config/__init__.py                            2      0   100%
	 normalization/transform_config/transform.py                         148     34    77%
	 -------------------------------------------------------------------------------------
	 TOTAL                                                              1258    525    58%

DoNotPanicUA · 2022-01-24T14:01:33Z

/test connector=connectors/destination-bigquery-denormalized

🕑 connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/1740237176
✅ connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/1740237176
Python tests coverage:

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                              Stmts   Miss  Cover
	 -------------------------------------------------------------------------------------
	 main_dev_transform_catalog.py                                         3      3     0%
	 main_dev_transform_config.py                                          3      3     0%
	 normalization/__init__.py                                             4      0   100%
	 normalization/destination_type.py                                    13      0   100%
	 normalization/transform_catalog/__init__.py                           2      0   100%
	 normalization/transform_catalog/catalog_processor.py                143     77    46%
	 normalization/transform_catalog/destination_name_transformer.py     155      8    95%
	 normalization/transform_catalog/reserved_keywords.py                 13      0   100%
	 normalization/transform_catalog/stream_processor.py                 520    333    36%
	 normalization/transform_catalog/table_name_registry.py              174     34    80%
	 normalization/transform_catalog/transform.py                         45     26    42%
	 normalization/transform_catalog/utils.py                             33      7    79%
	 normalization/transform_config/__init__.py                            2      0   100%
	 normalization/transform_config/transform.py                         148     34    77%
	 -------------------------------------------------------------------------------------
	 TOTAL                                                              1258    525    58%

DoNotPanicUA · 2022-01-24T14:01:43Z

/test connector=connectors/destination-snowflake

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

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                              Stmts   Miss  Cover
	 -------------------------------------------------------------------------------------
	 main_dev_transform_catalog.py                                         3      3     0%
	 main_dev_transform_config.py                                          3      3     0%
	 normalization/__init__.py                                             4      0   100%
	 normalization/destination_type.py                                    13      0   100%
	 normalization/transform_catalog/__init__.py                           2      0   100%
	 normalization/transform_catalog/catalog_processor.py                143     77    46%
	 normalization/transform_catalog/destination_name_transformer.py     155      8    95%
	 normalization/transform_catalog/reserved_keywords.py                 13      0   100%
	 normalization/transform_catalog/stream_processor.py                 520    333    36%
	 normalization/transform_catalog/table_name_registry.py              174     34    80%
	 normalization/transform_catalog/transform.py                         45     26    42%
	 normalization/transform_catalog/utils.py                             33      7    79%
	 normalization/transform_config/__init__.py                            2      0   100%
	 normalization/transform_config/transform.py                         148     34    77%
	 -------------------------------------------------------------------------------------
	 TOTAL                                                              1258    525    58%

DoNotPanicUA added 3 commits January 17, 2022 15:09

Interface reorganization

b6c5ffe

Merge remote-tracking branch 'origin/master' into aleonets/8894-merge…

0dc69d9

…-writer-interf

add missing impl

dcd945e

DoNotPanicUA self-assigned this Jan 18, 2022

github-actions bot added the area/connectors Connector related issues label Jan 18, 2022

DoNotPanicUA requested a review from alexandr-shegeda January 18, 2022 15:24

DoNotPanicUA added 2 commits January 18, 2022 17:27

Merge remote-tracking branch 'origin/master' into aleonets/8894-merge…

6eb1f71

…-writer-interf

Merge remote-tracking branch 'origin/master' into aleonets/8894-merge…

185ed58

…-writer-interf

DoNotPanicUA temporarily deployed to more-secrets January 20, 2022 11:30 Inactive

alexandr-shegeda requested review from alexandertsukanov and mkhokh-33 January 21, 2022 11:59

alexandr-shegeda reviewed Jan 21, 2022

View reviewed changes

Merge remote-tracking branch 'origin/master' into aleonets/8894-merge…

d5ce3d4

…-writer-interf

alexandr-shegeda linked an issue Jan 21, 2022 that may be closed by this pull request

Merge S3Writer, GcsWriter, CommonWriter interfaces #8894

Closed

mkhokh-33 suggested changes Jan 21, 2022

View reviewed changes

upd related classes

cf89b13

DoNotPanicUA temporarily deployed to more-secrets January 21, 2022 19:18 Inactive

alexandertsukanov approved these changes Jan 24, 2022

View reviewed changes

DoNotPanicUA added 2 commits January 24, 2022 15:10

Merge remote-tracking branch 'origin/master' into aleonets/8894-merge…

ca4e03e

…-writer-interf

review remarks

5244d52

octavia-squidington-iii temporarily deployed to more-secrets January 24, 2022 13:25 Inactive

DoNotPanicUA temporarily deployed to more-secrets January 24, 2022 14:02 Inactive

octavia-squidington-iii temporarily deployed to more-secrets January 24, 2022 14:03 Inactive

DoNotPanicUA requested review from mkhokh-33 and alexandr-shegeda January 24, 2022 17:29

DoNotPanicUA marked this pull request as ready for review January 24, 2022 17:29

mkhokh-33 approved these changes Jan 24, 2022

View reviewed changes

alexandr-shegeda approved these changes Jan 25, 2022

View reviewed changes

DoNotPanicUA merged commit ed1d26a into master Jan 25, 2022

DoNotPanicUA deleted the aleonets/8894-merge-writer-interf branch January 25, 2022 11:22

This was referenced Jan 25, 2022

Bump Airbyte version from 0.35.9-alpha to 0.35.10-alpha #9798

Closed

Bump Airbyte version from 0.35.9-alpha to 0.35.10-alpha #9800

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

S3, GCS Destinations : Fix interface duplication #9577

S3, GCS Destinations : Fix interface duplication #9577

DoNotPanicUA commented Jan 18, 2022 •

edited

Loading

alexandr-shegeda Jan 21, 2022

mkhokh-33 Jan 21, 2022

mkhokh-33 Jan 21, 2022

mkhokh-33 Jan 21, 2022

alexandertsukanov left a comment

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

S3, GCS Destinations : Fix interface duplication #9577

S3, GCS Destinations : Fix interface duplication #9577

Conversation

DoNotPanicUA commented Jan 18, 2022 • edited Loading

What

How

Recommended reading order

alexandr-shegeda Jan 21, 2022

Choose a reason for hiding this comment

mkhokh-33 Jan 21, 2022

Choose a reason for hiding this comment

mkhokh-33 Jan 21, 2022

Choose a reason for hiding this comment

mkhokh-33 Jan 21, 2022

Choose a reason for hiding this comment

alexandertsukanov left a comment

Choose a reason for hiding this comment

DoNotPanicUA commented Jan 24, 2022 • edited by github-actions bot Loading

DoNotPanicUA commented Jan 24, 2022 • edited Loading

DoNotPanicUA commented Jan 24, 2022 • edited by github-actions bot Loading

DoNotPanicUA commented Jan 24, 2022 • edited by github-actions bot Loading

DoNotPanicUA commented Jan 24, 2022 • edited by github-actions bot Loading

DoNotPanicUA commented Jan 24, 2022 • edited by github-actions bot Loading

DoNotPanicUA commented Jan 24, 2022 • edited by github-actions bot Loading

DoNotPanicUA commented Jan 24, 2022 • edited by github-actions bot Loading

DoNotPanicUA commented Jan 24, 2022 • edited by github-actions bot Loading

DoNotPanicUA commented Jan 24, 2022 • edited by github-actions bot Loading

DoNotPanicUA commented Jan 18, 2022 •

edited

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading

DoNotPanicUA commented Jan 24, 2022 •

edited by github-actions bot

Loading