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: Use Optional Instagram Business Account IDs. #22365

Closed
wants to merge 8 commits into from

Conversation

phani2410
Copy link

@phani2410 phani2410 commented Feb 3, 2023

What

Add Optional field to Instagram Source Config, for adding Instagram Business Account Ids.

How

Parse the Instagram Business Account IDs and filter the instagram business accounts that are available through the given access token. If no input is provided, all the instagram business accounts will be considered for the connection.
image

Recommended reading order

  1. airbyte-integrations/connectors/source-instagram/integration_tests/spec.json
  2. airbyte-integrations/connectors/source-instagram/source_instagram/source.py
  3. `airbyte-integrations/connectors/source-instagram/source_instagram/api.py'
  4. airbyte-integrations/connectors/source-instagram/unit_tests/conftest.py
  5. `docs/integrations/sources/instagram.md

🚨 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.

No Breaking changes. This will enable user to selectively stream data for given Instagram Account Ids (instead of all the connected business accounts available for the given access token)

Pre-merge Checklist

Expand the relevant checklist and delete the others.

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

Unit Tests

         ================= 77 passed, 35 warnings in 106.79s (0:01:46) ==================

> Task :airbyte-integrations:connectors:source-instagram:unitTest
Name                           Stmts   Miss  Cover
--------------------------------------------------
source_instagram/source.py        32      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           65      9    86%
--------------------------------------------------
TOTAL                            340     32    91%

Acceptance Tests

     ---------- coverage: platform darwin, python 3.9.6-final-0 -----------
     Name                                                 Stmts   Miss  Cover   Missing
     ----------------------------------------------------------------------------------
     source_acceptance_test/base.py                          12      4    67%   16-19
     source_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
     source_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
     source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
     source_acceptance_test/tests/test_core.py              476    117    75%   53, 58, 97-108, 113-120, 124-125, 129-130, 380, 400, 438, 476-493, 506-517, 521-526, 532, 565-570, 608-615, 658-660, 663, 728-736, 748-751, 756, 812-813, 819, 822, 858-868, 881-906
     source_acceptance_test/tests/test_incremental.py       160     14    91%   58-65, 70-83, 246
     source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
     source_acceptance_test/utils/common.py                  94      8    91%   32-38, 72, 75
     source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
     source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
     source_acceptance_test/utils/json_schema_helper.py     114     13    89%   31-32, 39, 42, 66-69, 97, 121, 203-205
     ----------------------------------------------------------------------------------
     TOTAL                                                 1690    339    80%

     5 files skipped due to complete coverage.

     Required test coverage of 74.0% reached. Total coverage: 79.94%

     Results (135.83s):
          475 passed

@CLAassistant
Copy link

CLAassistant commented Feb 3, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@juweins juweins left a comment

Choose a reason for hiding this comment

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

Hey @phani2410 ! Thank you for this addition and congratulations to your first contribution to airbyte!! 🎉

The code generally looks reasonable and good to me. However, can you please tell me why you used 10 / 17 digit regex at the highlighted positions? Why don't you use 15 digit to match the specs of a IG Business ID?

Copy link
Contributor

@juweins juweins left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you @phani2410

@sajarin @marcosmarxm it's your turn

@phani2410
Copy link
Author

@sajarin and @marcosmarxm, Please review this PR.

@phani2410
Copy link
Author

@sajarin and @marcosmarxm, Please review this PR.

@phani2410 phani2410 closed this Mar 23, 2023
@amrael
Copy link

amrael commented Jun 2, 2023

Why was the pull request closed? I need data from multiple accounts.

@phani2410
Copy link
Author

Reopening it.

@phani2410 phani2410 reopened this Jun 4, 2023
@phani2410 phani2410 closed this Jun 4, 2023
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 bounty community connectors/source/instagram
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants