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.

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

Source Airtable: skip missing streams #25946

Merged
merged 16 commits into from
May 18, 2023

Conversation

arsenlosenko
Copy link
Contributor

@arsenlosenko arsenlosenko commented May 10, 2023

What

Resolving issue described here:
#25547

How

Considering that the issue with missing streams most likely occurs if table on which stream is based on may be renamed/removed in Airtable I have added a check to take this into account. In case this happens, user will be presented with a warning, describing the issue and pointing to docs, and the stream in question will be removed from catalog till the data is reset.

🚨 User Impact 🚨

This is a patch change, tweaking the way read performs if the stream is missing

@arsenlosenko arsenlosenko self-assigned this May 10, 2023
@arsenlosenko arsenlosenko linked an issue May 10, 2023 that may be closed by this pull request
@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/airtable labels May 10, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 10, 2023

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 and you've followed all steps in the Breaking Changes Checklist
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • 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
  • You, or an Airbyter, have run /test successfully on this PR - or on a non-forked branch
  • You, or an Airbyter, have run /publish successfully on this PR - or on a non-forked branch
  • You've updated the connector's metadata.yaml file new!

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

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

@arsenlosenko
Copy link
Contributor Author

arsenlosenko commented May 10, 2023

/test connector=connectors/source-airtable

🕑 connectors/source-airtable https://github.com/airbytehq/airbyte/actions/runs/4935954337
❌ connectors/source-airtable https://github.com/airbytehq/airbyte/actions/runs/4935954337
🐛 https://gradle.com/s/cemvnn5k6xzhi

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestConnection::test_check[inputs2] - AssertionError: as...
FAILED test_core.py::TestDiscovery::test_discover[inputs1] - docker.errors.Co...
ERROR test_core.py::TestDiscovery::test_defined_cursors_exist_in_schema[inputs1]
ERROR test_core.py::TestDiscovery::test_defined_refs_exist_in_schema[inputs1]
ERROR test_core.py::TestDiscovery::test_defined_keyword_exist_in_schema[inputs1-allOf]
ERROR test_core.py::TestDiscovery::test_defined_keyword_exist_in_schema[inputs1-not]
ERROR test_core.py::TestDiscovery::test_primary_keys_exist_in_schema[inputs1]
ERROR test_core.py::TestDiscovery::test_streams_has_sync_modes[inputs1] - doc...
ERROR test_core.py::TestDiscovery::test_additional_properties_is_true[inputs1]
ERROR test_core.py::TestDiscovery::test_backward_compatibility[inputs1] - doc...
ERROR test_core.py::TestBasicRead::test_read[inputs1] - docker.errors.Contain...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: Incremental syncs are not supported on this connector.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:578: The previous and actual discovered catalogs are identical.
======== 2 failed, 38 passed, 3 skipped, 1 warning, 9 errors in 53.80s =========

@arsenlosenko
Copy link
Contributor Author

arsenlosenko commented May 11, 2023

/test connector=connectors/source-airtable

🕑 connectors/source-airtable https://github.com/airbytehq/airbyte/actions/runs/4950166791
❌ connectors/source-airtable https://github.com/airbytehq/airbyte/actions/runs/4950166791
🐛 https://gradle.com/s/bysuf7i2s4s32

Build Failed

Test summary info:

Could not find result summary

@arsenlosenko
Copy link
Contributor Author

arsenlosenko commented May 11, 2023

/test connector=connectors/source-airtable

🕑 connectors/source-airtable https://github.com/airbytehq/airbyte/actions/runs/4950343874
❌ connectors/source-airtable https://github.com/airbytehq/airbyte/actions/runs/4950343874
🐛 https://gradle.com/s/55n5d2755uvse

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestConnection::test_check[inputs2] - AssertionError: as...
FAILED test_core.py::TestDiscovery::test_discover[inputs1] - docker.errors.Co...
ERROR test_core.py::TestDiscovery::test_defined_cursors_exist_in_schema[inputs1]
ERROR test_core.py::TestDiscovery::test_defined_refs_exist_in_schema[inputs1]
ERROR test_core.py::TestDiscovery::test_defined_keyword_exist_in_schema[inputs1-allOf]
ERROR test_core.py::TestDiscovery::test_defined_keyword_exist_in_schema[inputs1-not]
ERROR test_core.py::TestDiscovery::test_primary_keys_exist_in_schema[inputs1]
ERROR test_core.py::TestDiscovery::test_streams_has_sync_modes[inputs1] - doc...
ERROR test_core.py::TestDiscovery::test_additional_properties_is_true[inputs1]
ERROR test_core.py::TestDiscovery::test_backward_compatibility[inputs1] - doc...
ERROR test_core.py::TestBasicRead::test_read[inputs1] - docker.errors.Contain...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: Incremental syncs are not supported on this connector.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:578: The previous and actual discovered catalogs are identical.
======== 2 failed, 38 passed, 3 skipped, 1 warning, 9 errors in 50.96s =========

@arsenlosenko
Copy link
Contributor Author

arsenlosenko commented May 11, 2023

/test connector=connectors/source-airtable

🕑 connectors/source-airtable https://github.com/airbytehq/airbyte/actions/runs/4950824209
✅ connectors/source-airtable https://github.com/airbytehq/airbyte/actions/runs/4950824209
Python tests coverage:

Name                                Stmts   Miss  Cover
-------------------------------------------------------
source_airtable/__init__.py             2      0   100%
source_airtable/streams.py             92      1    99%
source_airtable/source.py              53      3    94%
source_airtable/schema_helpers.py      48      9    81%
source_airtable/auth.py                21     10    52%
-------------------------------------------------------
TOTAL                                 216     23    89%

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: Incremental syncs are not supported on this connector.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:578: The previous and actual discovered catalogs are identical.
============= 48 passed, 4 skipped, 1 warning in 110.45s (0:01:50) =============

@arsenlosenko arsenlosenko requested a review from a team May 15, 2023 08:04
Copy link
Contributor

@bnchrch bnchrch left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

docs/integrations/sources/airtable.md Outdated Show resolved Hide resolved
docs/integrations/sources/airtable.md Outdated Show resolved Hide resolved
@arsenlosenko
Copy link
Contributor Author

arsenlosenko commented May 17, 2023

/test connector=connectors/source-airtable

🕑 connectors/source-airtable https://github.com/airbytehq/airbyte/actions/runs/5003696016
✅ connectors/source-airtable https://github.com/airbytehq/airbyte/actions/runs/5003696016
Python tests coverage:

Name                                Stmts   Miss  Cover
-------------------------------------------------------
source_airtable/__init__.py             2      0   100%
source_airtable/streams.py             92      1    99%
source_airtable/source.py              53      3    94%
source_airtable/schema_helpers.py      48      9    81%
source_airtable/auth.py                21     10    52%
-------------------------------------------------------
TOTAL                                 216     23    89%

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: Incremental syncs are not supported on this connector.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:578: The previous and actual discovered catalogs are identical.
============= 48 passed, 4 skipped, 1 warning in 144.22s (0:02:24) =============

@arsenlosenko
Copy link
Contributor Author

arsenlosenko commented May 17, 2023

/publish connector=connectors/source-airtable

🕑 Publishing the following connectors:
connectors/source-airtable
https://github.com/airbytehq/airbyte/actions/runs/5003913756


| Connector | Version | Did it publish? |
| --- | --- | --- | --- |
| connectors/source-airtable | 3.0.1 | ✅ |

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

@arsenlosenko arsenlosenko merged commit 9fb3542 into master May 18, 2023
17 of 18 checks passed
@arsenlosenko arsenlosenko deleted the arsenlosenko/25547-source-airtable-missing-stream branch May 18, 2023 09:15
edgao pushed a commit that referenced this pull request May 18, 2023
* Source Airtable: skip missing streams

* Move stream removal to a separate method, cover with tests

* Update changelog

* Fix flake warnings

* Update docs/integrations/sources/airtable.md

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

* Update docs/integrations/sources/airtable.md

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

* Automated Change

* Update link to docs in warning

* Automated Change

* Automated Change

* Automated Change

* “Empty-Commit”

---------

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
Co-authored-by: arsenlosenko <arsenlosenko@users.noreply.github.com>
edgao added a commit that referenced this pull request May 18, 2023
…from config (#26213)

* fix logic in parsing config

* simplify logic

* ugh

* holy moly that took way too many iterations

* version bumps / changelog

* Automated Change

* ✨ Destination Bigquery: stop running normalization container for DAT (#25925)

* readme update

* allow passing additional flags to test containers

* remove build dependency

* Automated Change

* versioning updates

* restore denormalized change from master

* formatting changes

* formatting

* Automated Change

* update metadata file

---------

Co-authored-by: jbfbell <jbfbell@users.noreply.github.com>

* fix version (#26218)

* Source Airtable: skip missing streams (#25946)

* Source Airtable: skip missing streams

* Move stream removal to a separate method, cover with tests

* Update changelog

* Fix flake warnings

* Update docs/integrations/sources/airtable.md

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

* Update docs/integrations/sources/airtable.md

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

* Automated Change

* Update link to docs in warning

* Automated Change

* Automated Change

* Automated Change

* “Empty-Commit”

---------

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
Co-authored-by: arsenlosenko <arsenlosenko@users.noreply.github.com>

* 🎉 New Source: Ringcentral [Low code CDK] (#25701)

* Initial commit - All test passed

* add stream fax cover

* refactor docs

* fix schema, Added pagination

* Add several streams, fix schema

* fix schema, add streams, refactor docs

* EOF

* Resolve conflicts

* Resolve conflicts

* add metadata file

---------

Co-authored-by: marcosmarxm <marcosmarxm@gmail.com>

* rebump version

* Automated Change

---------

Co-authored-by: edgao <edgao@users.noreply.github.com>
Co-authored-by: Joe Bell <joseph.bell@airbyte.io>
Co-authored-by: jbfbell <jbfbell@users.noreply.github.com>
Co-authored-by: Joe Reuter <joe@airbyte.io>
Co-authored-by: Arsen Losenko <20901439+arsenlosenko@users.noreply.github.com>
Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
Co-authored-by: arsenlosenko <arsenlosenko@users.noreply.github.com>
Co-authored-by: btkcodedev <btk.codedev@gmail.com>
Co-authored-by: marcosmarxm <marcosmarxm@gmail.com>
marcosmarxm pushed a commit to natalia-miinto/airbyte that referenced this pull request Jun 8, 2023
* Source Airtable: skip missing streams

* Move stream removal to a separate method, cover with tests

* Update changelog

* Fix flake warnings

* Update docs/integrations/sources/airtable.md

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

* Update docs/integrations/sources/airtable.md

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

* Automated Change

* Update link to docs in warning

* Automated Change

* Automated Change

* Automated Change

* “Empty-Commit”

---------

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
Co-authored-by: arsenlosenko <arsenlosenko@users.noreply.github.com>
marcosmarxm added a commit to natalia-miinto/airbyte that referenced this pull request Jun 8, 2023
…from config (airbytehq#26213)

* fix logic in parsing config

* simplify logic

* ugh

* holy moly that took way too many iterations

* version bumps / changelog

* Automated Change

* ✨ Destination Bigquery: stop running normalization container for DAT (airbytehq#25925)

* readme update

* allow passing additional flags to test containers

* remove build dependency

* Automated Change

* versioning updates

* restore denormalized change from master

* formatting changes

* formatting

* Automated Change

* update metadata file

---------

Co-authored-by: jbfbell <jbfbell@users.noreply.github.com>

* fix version (airbytehq#26218)

* Source Airtable: skip missing streams (airbytehq#25946)

* Source Airtable: skip missing streams

* Move stream removal to a separate method, cover with tests

* Update changelog

* Fix flake warnings

* Update docs/integrations/sources/airtable.md

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

* Update docs/integrations/sources/airtable.md

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

* Automated Change

* Update link to docs in warning

* Automated Change

* Automated Change

* Automated Change

* “Empty-Commit”

---------

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
Co-authored-by: arsenlosenko <arsenlosenko@users.noreply.github.com>

* 🎉 New Source: Ringcentral [Low code CDK] (airbytehq#25701)

* Initial commit - All test passed

* add stream fax cover

* refactor docs

* fix schema, Added pagination

* Add several streams, fix schema

* fix schema, add streams, refactor docs

* EOF

* Resolve conflicts

* Resolve conflicts

* add metadata file

---------

Co-authored-by: marcosmarxm <marcosmarxm@gmail.com>

* rebump version

* Automated Change

---------

Co-authored-by: edgao <edgao@users.noreply.github.com>
Co-authored-by: Joe Bell <joseph.bell@airbyte.io>
Co-authored-by: jbfbell <jbfbell@users.noreply.github.com>
Co-authored-by: Joe Reuter <joe@airbyte.io>
Co-authored-by: Arsen Losenko <20901439+arsenlosenko@users.noreply.github.com>
Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
Co-authored-by: arsenlosenko <arsenlosenko@users.noreply.github.com>
Co-authored-by: btkcodedev <btk.codedev@gmail.com>
Co-authored-by: marcosmarxm <marcosmarxm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/airtable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Source: Airtable - stream missing during sync
5 participants