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 GitLab: Accept api_url with or without scheme #21373

Merged
merged 21 commits into from Apr 27, 2023

Conversation

pgollangi
Copy link
Contributor

@pgollangi pgollangi commented Jan 13, 2023

Make source-gitlab to accept api_urls with or without scheme, also accepts the URL with API endpoint included init. Currently gitlab source ONLY accept urls without scheme.

Also fixed issues with running unit tests (missing dependency) see #21372

Fixes #21143 and #21372

User Impact

This change will not break any of the existing GitLab configuration, but starts accepting gitlab urls without HTTP or HTTPs schemes.

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

airbyte-integrations/connectors/source-gitlab/unit_tests/test_source.py::test_streams[gitlab.com]
airbyte-integrations/connectors/source-gitlab/unit_tests/test_source.py::test_streams[https://gitlab.com]
airbyte-integrations/connectors/source-gitlab/unit_tests/test_source.py::test_streams[https://gitlab.com/api/v4]
airbyte-integrations/connectors/source-gitlab/unit_tests/test_source.py::test_connection_success[gitlab.com]
airbyte-integrations/connectors/source-gitlab/unit_tests/test_source.py::test_connection_success[https://gitlab.com]
airbyte-integrations/connectors/source-gitlab/unit_tests/test_source.py::test_connection_success[https://gitlab.com/api/v4]
airbyte-integrations/connectors/source-gitlab/unit_tests/test_source.py::test_connection_fail[gitlab.com]
airbyte-integrations/connectors/source-gitlab/unit_tests/test_source.py::test_connection_fail[https://gitlab.com]
airbyte-integrations/connectors/source-gitlab/unit_tests/test_source.py::test_connection_fail[https://gitlab.com/api/v4]
  /Users/p.gollangi/dev/pgollangi-github/airbyte/airbyte-integrations/connectors/source-gitlab/source_gitlab/source.py:70: DeprecationWarning: Call to deprecated class TokenAuthenticator. (Use airbyte_cdk.sources.streams.http.requests_native_auth.TokenAuthenticator instead) -- Deprecated since version 0.1.20.
    auth = TokenAuthenticator(token=config["private_token"])

-- Docs: https://docs.pytest.org/en/stable/warnings.html
======================================================================================= 20 passed, 198 warnings in 0.35s =======================================================================================
Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

@CLAassistant
Copy link

CLAassistant commented Jan 13, 2023

CLA assistant check
All committers have signed the CLA.

@pgollangi
Copy link
Contributor Author

@yevhenii-ldv @ykurochkin @juweins could you please take a look at this? related to #21143 and #21372

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

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.

Took some time for me to come back to this issue! Your adjustments LGTM and are reasonable. Thank you @pgollangi

pgollangi and others added 3 commits January 26, 2023 20:56
* WIP

* WIP

* wip

* more fixes

* fix test

* remove some more styled components and improve memoization

* review comment
@octavia-squidington-iv octavia-squidington-iv added the area/frontend Related to the Airbyte webapp label Jan 26, 2023
@octavia-squidington-iv octavia-squidington-iv removed the area/frontend Related to the Airbyte webapp label Jan 26, 2023
@pgollangi
Copy link
Contributor Author

@juweins thanks for the review. I see my pipelines are failing, thus merge is blocked. Any clue on this? https://github.com/airbytehq/airbyte/actions/runs/4017449728/jobs/6901816873

@juweins
Copy link
Contributor

juweins commented Jan 31, 2023

@juweins thanks for the review. I see my pipelines are failing, thus merge is blocked. Any clue on this? https://github.com/airbytehq/airbyte/actions/runs/4017449728/jobs/6901816873
@pgollangi
Although I gave my apporve, it seems like I don't actaully have the permission to ultimately do that? Maybe @marcosmarxm can shed light into this.

@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Apr 20, 2023
@archangelic
Copy link
Contributor

archangelic commented Apr 21, 2023

/test connector=connectors/source-gitlab

🕑 connectors/source-gitlab https://github.com/airbytehq/airbyte/actions/runs/4768356208
❌ connectors/source-gitlab https://github.com/airbytehq/airbyte/actions/runs/4768356208
🐛 https://gradle.com/s/krxky3bynn2py

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestConnection::test_check[inputs1] - AssertionError: as...
ERROR test_core.py::TestBasicRead::test_read[inputs2] - docker.errors.Contain...
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.
========= 1 failed, 41 passed, 2 skipped, 1 error in 477.24s (0:07:57) =========

@marcosmarxm
Copy link
Member

marcosmarxm commented Apr 25, 2023

/test connector=connectors/source-gitlab

🕑 connectors/source-gitlab https://github.com/airbytehq/airbyte/actions/runs/4801302230
❌ connectors/source-gitlab https://github.com/airbytehq/airbyte/actions/runs/4801302230
🐛 https://gradle.com/s/ac2p745jlxjdo

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestConnection::test_check[inputs1] - AssertionError: as...
ERROR test_core.py::TestBasicRead::test_read[inputs2] - docker.errors.Contain...
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.
========= 1 failed, 41 passed, 2 skipped, 1 error in 427.68s (0:07:07) =========

@marcosmarxm
Copy link
Member

marcosmarxm commented Apr 27, 2023

/test connector=connectors/source-gitlab

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

Name                        Stmts   Miss  Cover
-----------------------------------------------
source_gitlab/__init__.py       2      0   100%
source_gitlab/source.py        61      3    95%
source_gitlab/streams.py      279     15    95%
-----------------------------------------------
TOTAL                         342     18    95%

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: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.
================== 43 passed, 2 skipped in 609.07s (0:10:09) ===================

@archangelic
Copy link
Contributor

archangelic commented Apr 27, 2023

/publish connector=connectors/source-gitlab

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


Connector Version Did it publish? Were definitions generated?
connectors/source-gitlab 1.0.4

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

@archangelic archangelic mentioned this pull request Apr 27, 2023
@pgollangi
Copy link
Contributor Author

@archangelic would you mind approve again. Github tests were failing on my branch. Latest changes from main merged to my feature branch.

@archangelic archangelic mentioned this pull request Apr 27, 2023
@archangelic archangelic merged commit e2b46dd into airbytehq:master Apr 27, 2023
32 of 37 checks passed
marcosmarxm added a commit to natalia-miinto/airbyte that referenced this pull request Jun 8, 2023
* feat: source-gitlab accept api_url w/ or w/o scheme

* 🪟🔧 Refactor ConnectorsView (airbytehq#21531)

* WIP

* WIP

* wip

* more fixes

* fix test

* remove some more styled components and improve memoization

* review comment

* feat: source-gitlab accept api_url w/ or w/o scheme

* Revert allowedHosts missing value checks (airbytehq#21923)

* bump versions

* update expected records

* auto-bump connector version

---------

Co-authored-by: Joe Reuter <joe@airbyte.io>
Co-authored-by: Evan Tahler <evan@airbyte.io>
Co-authored-by: Mal Hancock <mallory@archangelic.space>
Co-authored-by: Marcos Marx <marcosmarxm@users.noreply.github.com>
Co-authored-by: marcosmarxm <marcosmarxm@gmail.com>
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
Projects
Development

Successfully merging this pull request may close these issues.

Source Gitlab: Explicitly state api_url accepts URL without scheme (http or https)
8 participants