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 Github: add new streams TeamMembers, TeamMemberships #11893

Merged
merged 22 commits into from
Apr 21, 2022

Conversation

grubberr
Copy link
Contributor

@grubberr grubberr commented Apr 11, 2022

Signed-off-by: Sergey Chvalyuk grubberr@gmail.com

What

Source Github: add new streams TeamMembers, TeamMemberships

How

Describe the solution

Recommended reading order

  1. x.java
  2. y.python

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

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

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

┆Issue is synchronized with this Monday item by Unito

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr grubberr self-assigned this Apr 11, 2022
@github-actions github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Apr 11, 2022
@grubberr grubberr linked an issue Apr 11, 2022 that may be closed by this pull request
@grubberr grubberr temporarily deployed to more-secrets April 11, 2022 19:51 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 11, 2022 19:51 Inactive
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr grubberr temporarily deployed to more-secrets April 11, 2022 19:55 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 11, 2022 19:55 Inactive
@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

❗ No coverage uploaded for pull request base (master@aa872d0). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #11893   +/-   ##
=========================================
  Coverage          ?   91.04%           
=========================================
  Files             ?        3           
  Lines             ?      614           
  Branches          ?        0           
=========================================
  Hits              ?      559           
  Misses            ?       55           
  Partials          ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa872d0...036f9db. Read the comment docs.

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr
Copy link
Contributor Author

grubberr commented Apr 11, 2022

/test connector=connectors/source-github

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

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              285    106    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  886    259    71%
Name                        Stmts   Miss  Cover
-----------------------------------------------
source_github/__init__.py       2      0   100%
source_github/streams.py      500     40    92%
source_github/source.py        90     25    72%
-----------------------------------------------
TOTAL                         592     65    89%

@grubberr grubberr temporarily deployed to more-secrets April 11, 2022 20:10 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 11, 2022 20:10 Inactive
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr grubberr changed the title 🐛 Source Github: add new stream TeamMembers 🐛 Source Github: add new streams TeamMembers, TeamMemberships Apr 12, 2022
@grubberr grubberr temporarily deployed to more-secrets April 12, 2022 08:19 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 12, 2022 08:19 Inactive
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr
Copy link
Contributor Author

grubberr commented Apr 12, 2022

/test connector=connectors/source-github

🕑 connectors/source-github https://github.com/airbytehq/airbyte/actions/runs/2153640381
❌ connectors/source-github https://github.com/airbytehq/airbyte/actions/runs/2153640381
🐛 https://gradle.com/s/cz47a2qgko2d6
Python short test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestDiscovery::test_primary_keys_exist_in_schema[inputs0]
FAILED test_core.py::TestDiscovery::test_primary_keys_exist_in_schema[inputs1]
FAILED test_core.py::TestBasicRead::test_read[inputs0] - AssertionError: Prim...
=================== 3 failed, 26 passed in 167.86s (0:02:47) ===================

@grubberr grubberr temporarily deployed to more-secrets April 12, 2022 08:22 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 12, 2022 08:22 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 13, 2022 08:47 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 13, 2022 08:47 Inactive
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr grubberr temporarily deployed to more-secrets April 13, 2022 10:45 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 13, 2022 10:45 Inactive
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr grubberr temporarily deployed to more-secrets April 13, 2022 19:54 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 13, 2022 19:54 Inactive
@grubberr
Copy link
Contributor Author

grubberr commented Apr 14, 2022

/test connector=connectors/source-github

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

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              285    106    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  886    259    71%
Name                        Stmts   Miss  Cover
-----------------------------------------------
source_github/__init__.py       2      0   100%
source_github/streams.py      521     29    94%
source_github/source.py        91     26    71%
-----------------------------------------------
TOTAL                         614     55    91%

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr grubberr temporarily deployed to more-secrets April 14, 2022 05:24 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 14, 2022 05:25 Inactive

def __init__(self, parent: HttpStream, **kwargs):
super().__init__(**kwargs)
self.parent = parent
Copy link
Contributor

Choose a reason for hiding this comment

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

as for me, I would better declare parent class directly in this class. like it is done in another streams with 'parent_entity'

Passing 'parent' class as an argument assumes that parent can be different. But here we should preciscly define that we need only Teams stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes I thought about it
but because there are more then one level of nesting of classes:

  1. TeamMembers -> Teams
  2. TeamMemberships -> TeamMembers -> Teams

I decided just to keep this approach
some old classed also use this approach

I changed type annotation to be more specific
8411b31

@grubberr grubberr temporarily deployed to more-secrets April 19, 2022 15:38 Inactive
@grubberr grubberr temporarily deployed to more-secrets April 19, 2022 15:38 Inactive
@grubberr
Copy link
Contributor Author

grubberr commented Apr 19, 2022

/test connector=connectors/source-github

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

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              285    106    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  886    259    71%
Name                        Stmts   Miss  Cover
-----------------------------------------------
source_github/__init__.py       2      0   100%
source_github/streams.py      521     29    94%
source_github/source.py        91     26    71%
-----------------------------------------------
TOTAL                         614     55    91%

@grubberr grubberr requested a review from midavadim April 19, 2022 16:57
@midavadim
Copy link
Contributor

Can be released without airbyte review

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr
Copy link
Contributor Author

grubberr commented Apr 21, 2022

/publish connector=connectors/source-github

🕑 connectors/source-github https://github.com/airbytehq/airbyte/actions/runs/2202735815
🚀 Successfully published connectors/source-github
🚀 Auto-bumped version for connectors/source-github
✅ connectors/source-github https://github.com/airbytehq/airbyte/actions/runs/2202735815

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets April 21, 2022 15:34 Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets April 21, 2022 15:34 Inactive
@grubberr grubberr merged commit 8cf4569 into master Apr 21, 2022
@grubberr grubberr deleted the grubberr/11886-source-github branch April 21, 2022 15:50
suhomud pushed a commit that referenced this pull request May 23, 2022
Signed-off-by: Sergey Chvalyuk <grubberr@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Source Github: add new streams TeamMembers, TeamMemberships
3 participants