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 Instagram: decrease give up rate #23671

Merged
merged 7 commits into from
Mar 20, 2023

Conversation

darynaishchenko
Copy link
Collaborator

Give up reason: the endpoint requires the pages_read_engagement permission. For now we haven't an access to our test account to test these permissions(add/remove). See here for more info.
Phase 1, in this PR was added info about main permissions in spec and added doc links in error message to navigate user.

@darynaishchenko darynaishchenko self-assigned this Mar 2, 2023
@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/instagram labels Mar 2, 2023
@darynaishchenko
Copy link
Collaborator Author

darynaishchenko commented Mar 2, 2023

/test connector=connectors/source-instagram

🕑 connectors/source-instagram https://github.com/airbytehq/airbyte/actions/runs/4315613404
❌ connectors/source-instagram https://github.com/airbytehq/airbyte/actions/runs/4315613404
🐛 https://gradle.com/s/nmfpwrvpzuag2

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Stream user_...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_incremental.py:212: Skipping new incremental test based on acceptance-test-config.yml
============= 1 failed, 34 passed, 2 skipped in 304.40s (0:05:04) ==============

@darynaishchenko
Copy link
Collaborator Author

darynaishchenko commented Mar 9, 2023

/test connector=connectors/source-instagram

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

Name                           Stmts   Miss  Cover
--------------------------------------------------
source_instagram/source.py        31      0   100%
source_instagram/__init__.py       2      0   100%
source_instagram/common.py        36      3    92%
source_instagram/streams.py      205     20    90%
source_instagram/api.py           59      7    88%
--------------------------------------------------
TOTAL                            333     30    91%
Name                           Stmts   Miss  Cover
--------------------------------------------------
source_instagram/__init__.py       2      0   100%
source_instagram/api.py           59      5    92%
source_instagram/source.py        31     11    65%
source_instagram/streams.py      205     94    54%
source_instagram/common.py        36     22    39%
--------------------------------------------------
TOTAL                            333    132    60%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_incremental.py:212: Skipping new incremental test based on acceptance-test-config.yml
================== 35 passed, 2 skipped in 225.34s (0:03:45) ===================

@darynaishchenko
Copy link
Collaborator Author

darynaishchenko commented Mar 16, 2023

/test connector=connectors/source-instagram

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

Name                           Stmts   Miss  Cover
--------------------------------------------------
source_instagram/source.py        31      0   100%
source_instagram/__init__.py       2      0   100%
source_instagram/common.py        36      3    92%
source_instagram/streams.py      215     20    91%
source_instagram/api.py           59      7    88%
--------------------------------------------------
TOTAL                            343     30    91%
Name                           Stmts   Miss  Cover
--------------------------------------------------
source_instagram/__init__.py       2      0   100%
source_instagram/api.py           59      5    92%
source_instagram/source.py        31     11    65%
source_instagram/streams.py      215     99    54%
source_instagram/common.py        36     22    39%
--------------------------------------------------
TOTAL                            343    137    60%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_incremental.py:212: Skipping new incremental test based on acceptance-test-config.yml
================== 35 passed, 2 skipped in 203.27s (0:03:23) ===================

Copy link
Contributor

@erohmensing erohmensing left a comment

Choose a reason for hiding this comment

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

One question, but 👍🏻

@@ -88,7 +88,13 @@ def _find_accounts(self) -> List[Mapping[str, Any]]:
}
)
except FacebookRequestError as exc:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do all FacebookRequestErrors we might run into here pertain to permissions issues? Is there something more specific we could except in order to raise this specific error message in order to avoid accidentally masking other issues as permission issues?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks for commenting, I added improvement for this code, now we checking the error status code and if this code rated to permissions we also say about checking permission needed for our connector.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, thanks!

@darynaishchenko
Copy link
Collaborator Author

darynaishchenko commented Mar 20, 2023

/test connector=connectors/source-instagram

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

Name                           Stmts   Miss  Cover
--------------------------------------------------
source_instagram/source.py        31      0   100%
source_instagram/__init__.py       2      0   100%
source_instagram/common.py        36      3    92%
source_instagram/streams.py      215     20    91%
source_instagram/api.py           61      9    85%
--------------------------------------------------
TOTAL                            345     32    91%
Name                           Stmts   Miss  Cover
--------------------------------------------------
source_instagram/__init__.py       2      0   100%
source_instagram/api.py           61      7    89%
source_instagram/source.py        31     11    65%
source_instagram/streams.py      215     99    54%
source_instagram/common.py        36     22    39%
--------------------------------------------------
TOTAL                            345    139    60%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_incremental.py:212: Skipping new incremental test based on acceptance-test-config.yml
================== 35 passed, 2 skipped in 173.53s (0:02:53) ===================

@darynaishchenko
Copy link
Collaborator Author

darynaishchenko commented Mar 20, 2023

/publish connector=connectors/source-instagram

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


Connector Did it publish? Were definitions generated?
connectors/source-instagram

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

@darynaishchenko darynaishchenko merged commit edf5b19 into master Mar 20, 2023
@darynaishchenko darynaishchenko deleted the daryna/source-instagram/decrase-give-up-rate branch March 20, 2023 15:19
erohmensing pushed a commit that referenced this pull request Mar 22, 2023
* updated spec and added more specific error message

* added change log

* refactored error handling

* auto-bump connector version

---------

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
erohmensing pushed a commit that referenced this pull request Mar 22, 2023
* updated spec and added more specific error message

* added change log

* refactored error handling

* auto-bump connector version

---------

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.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/instagram
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants