✨CAT: Add test to ensure all file types covered #33746

askarpets · 2023-12-22T13:00:49Z

What

Add a CAT to ensure all file types covered
Resolves #33363

How

This test will run if connector's metadata has the following params:

connectorSubtype: file
ab_internal.ql >= 400

(i.e. certified file-based connectors) and verify that the sandbox account for this connector contains the following:

all supported structured file types: .avro, .csv, .jsonl, .parquet
at least one of supported unstructured file types: .pdf, .doc, .docx, .ppt, .pptx, .md

unless otherwise specified in the connector's tests config (acceptance-test-config.yml).

In case the connector does not support some of the file types listed above, there is a possibility to disable checks for them using unsupported_types section in config:

acceptance_tests:
  basic_read:
    tests:
      - config_path: secrets/config.json
        expect_records:
          path: integration_tests/expected_records/csv.jsonl
          exact_order: true
        file_types:
          unsupported_types:
            - extension: .csv
              bypass_reason: "Optional reason of why this type is not supported"
            - extension: .avro

Another option is to skip the test at all:

acceptance_tests:
  basic_read:
    tests:
      - config_path: secrets/config.json
        expect_records:
          path: integration_tests/expected_records/csv.jsonl
          exact_order: true
        file_types:
          skip_test: true
          bypass_reason: "Optional reason of why this test is skipped"

Important note

This test collects available files and their corresponding types in TestBasicRead.test_read (to avoid extra API calls), so if your connector have multiple basic_read configs for different file types (e.g. source-s3), please bypass all configs except the last one.

Next steps

The following connectors have connectorSubtype: file and ab_internal.ql >= 400 and their configs need to be reviewed:

source-file
source-google-sheets
source-s3

🚨 User Impact 🚨

No breaking changes

Pre-merge Actions

Updating the Python CDK

Airbyter

Before merging:

Pull Request description explains what problem it is solving
Code change is unit tested
Build and my-py check pass
Smoke test the change on at least one affected connector
- On Github: Run this workflow, passing --use-local-cdk --name=source-<connector> as options
- Locally: airbyte-ci connectors --use-local-cdk --name=source-<connector> test
PR is reviewed and approved

After merging:

Publish the CDK
- The CDK does not follow proper semantic versioning. Choose minor if this the change has significant user impact or is a breaking change. Choose patch otherwise.
- Write a thoughtful changelog message so we know what was updated.
Merge the platform PR that was auto-created for updating the Connector Builder's CDK version
- This step is optional if the change does not affect the connector builder or declarative connectors.

vercel · 2023-12-22T13:00:55Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	⬜️ Ignored (Inspect)	Visit Preview		Jan 26, 2024 10:36am

github-actions · 2023-12-22T13:01:04Z

Warning

Soft code freeze is in effect until 2024-01-02. Please avoid merging to master. #freedom-and-responsibility

lazebnyi

LGTM

clnoll

Thanks @askarpets - just a few requests.

In addition to the requests inline in the code, can you please update the PR description with:

An example of how a user can set unsupported file types
An example of how a user can bypass this test
A list of which connectors we expect to run this test by default (i.e. a list of file-based, certified connectors). If we expect any to fail because the test config needs to be updated to list the unsupported file types, please state that here and either open a ticket or go ahead and update the config.

clnoll · 2024-01-16T15:08:56Z

airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/config.py

+    file_types: Optional[FileTypesConfig] = Field(
+        default_factory=FileTypesConfig,
+        description="For file-based connectors, unsupported by source file types can be configured or a test can be skipped at all",
+    )


I'm wondering if this should just be called unsupported_file_types, and should be a list/set (similar to empty_streams). WDYT?

Hi @clnoll, thanks for the review!
I added this section as a complex object mostly because we need to have the ability to skip this test at all, and to do so, a user just need to set FileTypesConfig.skip_test = True instead of listing all available file types. Do you think there could be a better approach?

Okay, I see what you mean now. LGTM!

clnoll · 2024-01-16T15:18:18Z

...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py

+    @staticmethod
+    def _get_file_extension(file_name: str) -> str:
+        _, file_extension = splitext(file_name)
+        return file_extension


Suggested change

return file_extension

return file_extension.casefold()

Good suggestion, thanks! I also added this to _get_unsupported_file_types method to avoid issues when users list unsupported file types in uppercase for some reason.

clnoll · 2024-01-16T15:20:08Z

...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py

+            f"Please make sure you added files with all of supported structured types {structured_types} "
+            f"and at least one with unstructured type {unstructured_types} to the test account."


Would you mind updating this to 1) list out the ones that are missing, and 2) provide instructions for marking a file type as unsupported?

Updated, please take a look

Thank you! Looks great.

clnoll

LGTM!

This reverts commit 5d0fc4d.

CAT: Add test to ensure all file types covered

5fb2a27

askarpets self-assigned this Dec 22, 2023

askarpets added 3 commits December 27, 2023 13:52

Merge branch 'master' into cat-test-all-supported-file-types-present

308720e

Add unit tests

a363bb5

Add unit tests

17db664

askarpets marked this pull request as ready for review December 27, 2023 15:49

askarpets requested review from lazebnyi and oustynova as code owners December 27, 2023 15:49

askarpets requested a review from a team December 27, 2023 15:49

lazebnyi approved these changes Jan 9, 2024

View reviewed changes

askarpets requested review from alafanechere and girarda January 9, 2024 13:41

octavia-squidington-iv requested a review from a team January 11, 2024 13:43

girarda requested a review from clnoll January 11, 2024 15:02

clnoll reviewed Jan 16, 2024

View reviewed changes

katmarkham removed the request for review from a team January 17, 2024 18:05

askarpets added 3 commits January 25, 2024 17:45

Merge branch 'master' into cat-test-all-supported-file-types-present

7bbb7ad

Update logic of checking if connector is certified

9526ad7

Update changelog

0bf41ee

askarpets requested a review from clnoll January 25, 2024 18:37

Fix tests

75a9f7e

clnoll approved these changes Jan 25, 2024

View reviewed changes

askarpets added 6 commits January 26, 2024 11:39

Merge branch 'master' into cat-test-all-supported-file-types-present

57b88ce

Update docs

af23f1b

Update CAT config for source-s3

1752a80

Update CAT config for source-google-sheets

8e54fab

Update version in pyproject.toml

2dad60e

Update CAT config for source-file

5d0fc4d

octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/file connectors/source/google-sheets connectors/source/s3 labels Jan 26, 2024

Revert "Update CAT config for source-file"

e722282

This reverts commit 5d0fc4d.

askarpets merged commit 3592ee9 into master Jan 26, 2024
29 of 30 checks passed

askarpets deleted the cat-test-all-supported-file-types-present branch January 26, 2024 10:52

jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 21, 2024

✨CAT: Add test to ensure all file types covered (airbytehq#33746)

be91438

jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024

✨CAT: Add test to ensure all file types covered (airbytehq#33746)

e764553

jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024

✨CAT: Add test to ensure all file types covered (airbytehq#33746)

c5345a2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨CAT: Add test to ensure all file types covered #33746

✨CAT: Add test to ensure all file types covered #33746

askarpets commented Dec 22, 2023 •

edited

Loading

vercel bot commented Dec 22, 2023 •

edited

Loading

github-actions bot commented Dec 22, 2023

lazebnyi left a comment

clnoll left a comment

clnoll Jan 16, 2024

askarpets Jan 25, 2024

clnoll Jan 25, 2024

clnoll Jan 16, 2024

askarpets Jan 25, 2024

clnoll Jan 16, 2024

askarpets Jan 25, 2024

clnoll Jan 25, 2024

clnoll left a comment

		f"Please make sure you added files with all of supported structured types {structured_types} "
		f"and at least one with unstructured type {unstructured_types} to the test account."

✨CAT: Add test to ensure all file types covered #33746

✨CAT: Add test to ensure all file types covered #33746

Conversation

askarpets commented Dec 22, 2023 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Actions

Airbyter

vercel bot commented Dec 22, 2023 • edited Loading

github-actions bot commented Dec 22, 2023

lazebnyi left a comment

Choose a reason for hiding this comment

clnoll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clnoll left a comment

Choose a reason for hiding this comment

askarpets commented Dec 22, 2023 •

edited

Loading

vercel bot commented Dec 22, 2023 •

edited

Loading