-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: Fix syncing branches for the project with Repository feature disabled #26422
🐛Source GitLab: Fix syncing branches for the project with Repository feature disabled #26422
Conversation
Before Merging a Connector Pull RequestWow! 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:
If the checklist is complete, but the CI check is failing,
|
yield from super().read_records(sync_mode, cursor_field, stream_slice, stream_state) | ||
except requests.exceptions.HTTPError as error: | ||
status = error.response.status_code | ||
if status == 404: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already do have handling of 403 error, could it be extended to also cope with 404?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because this fix is done only for branches stream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could make it customizable per stream, for instance:
class GitlabStream:
non_retriable_codes: List[int] = [403]
...
...
if response.status_code in self.non_retriable_codes:
setattr(self, "raise_on_http_errors", False)
self.logger.warning(
f"Got 403 error when accessing URL {response.request.url}."
f" Very likely the feature is disabled for this project and/or group. Please double check it, or report a bug otherwise.")
return False
return super().should_retry(response)
class Branches:
non_retriable_codes = [403, 404]
@@ -86,7 +87,7 @@ def next_page_token(self, response: requests.Response) -> Optional[Mapping[str, | |||
return {"page": self.page} | |||
|
|||
def parse_response(self, response: requests.Response, **kwargs) -> Iterable[Mapping]: | |||
if response.status_code == 403: | |||
if response.status_code in [403, 404]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if response.status_code in [403, 404]: | |
if response.status_code != 200: |
/test connector=connectors/source-gitlab
Build PassedTest summary info:
|
…en-syncing-branch-from-repository-feature-disabled
…feature disabled (airbytehq#26422) Add handling blocked repository for branches stream
What
Fix syncing branches for the project with the Repository feature disabled
#23130
How
Added try..except for read_records of branches stream
Tests
Test for repository feature disabled
airbyte/airbyte-integrations/connectors/source-gitlab# python main.py read --config secrets/config.json --catalog integration_tests/configured_catalog_branches.json
{"type": "LOG", "log": {"level": "INFO", "message": "Starting syncing SourceGitlab"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Marking stream branches as STARTED"}}
{"type": "TRACE", "trace": {"type": "STREAM_STATUS", "emitted_at": 1684844308347.508, "stream_status": {"stream_descriptor": {"name": "branches", "namespace": null}, "status": "STARTED"}}}
{"type": "LOG", "log": {"level": "INFO", "message": "Syncing stream: branches "}}
{"type": "LOG", "log": {"level": "ERROR", "message": "{"message":"404 Repository Not Found"}"}}
{"type": "LOG", "log": {"level": "WARN", "message": "Got 404 error when accessing branches. Very likely the feature is disabled for this project and/or group. Please double check it, or report a bug otherwise."}}
{"type": "LOG", "log": {"level": "INFO", "message": "Read 0 records from branches stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Marking stream branches as STOPPED"}}
{"type": "TRACE", "trace": {"type": "STREAM_STATUS", "emitted_at": 1684844314462.594, "stream_status": {"stream_descriptor": {"name": "branches", "namespace": null}, "status": "COMPLETE"}}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing branches"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceGitlab runtimes:\nSyncing stream branches 0:00:06.115927"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceGitlab"}}
Integration
airbyte/airbyte# ./gradlew :airbyte-integrations:connectors:source-gitlab:integrationTest Starting a Gradle Daemon (subsequent builds will be faster) Building all of Airbyte. /home/anatolii/airbyte/airbyte/airbyte-integrations/connectors
source_gitlab/init.py 2 0 100%
source_gitlab/source.py 61 3 95%
source_gitlab/streams.py 288 16 94%
TOTAL 351 19 95%
#2 [internal] load build definition from Dockerfile
#2 transferring dockerfile: 513B 0.0s done
#2 DONE 0.1s
#3 [internal] load metadata for docker.io/library/python:3.9-slim
#3 DONE 2.4s
#4 [internal] load build context
#4 transferring context: 1.83kB 0.0s done
#4 DONE 0.0s
#5 [1/7] FROM docker.io/library/python:3.9-slim@sha256:4b1d0686095059e04d5c0fc3c6a67813b1d9f1428e8ae1d257b95e6b185ed144
#5 resolve docker.io/library/python:3.9-slim@sha256:4b1d0686095059e04d5c0fc3c6a67813b1d9f1428e8ae1d257b95e6b185ed144
#5 resolve docker.io/library/python:3.9-slim@sha256:4b1d0686095059e04d5c0fc3c6a67813b1d9f1428e8ae1d257b95e6b185ed144 0.1s done
#5 DONE 0.1s
#6 [2/7] RUN apt-get update && apt-get install -y bash && rm -rf /var/lib/apt/lists/*
#6 CACHED
#7 [3/7] WORKDIR /airbyte/integration_code
#7 CACHED
#8 [4/7] COPY setup.py ./
#8 CACHED
#9 [5/7] RUN pip install .
#9 CACHED
#10 [6/7] COPY source_gitlab ./source_gitlab
#10 CACHED
#11 [7/7] COPY main.py ./
#11 CACHED
#12 exporting to image
#12 exporting layers done
#12 writing image sha256:7eb40972eec3ea7e66e43b30d90c70dc7afb18ceddf12cdae4e09adbace0d52a done
#12 naming to docker.io/airbyte/source-gitlab:dev done
#12 DONE 0.0s
test_core.py .....................s............s...... [ 91%]
test_full_refresh.py . [ 93%]
test_incremental.py ... [100%]
=========================== 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 1133.91s (0:18:53) ==================
Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
See https://docs.gradle.org/7.6/userguide/command_line_interface.html#sec:command_line_warnings
BUILD SUCCESSFUL in 19m 53s
27 actionable tasks: 9 executed, 18 up-to-date