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: log the error message for streams that are restricted for OAuth #9868

Merged
merged 7 commits into from
Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"sourceDefinitionId": "ef69ef6e-aa7f-4af1-a01d-ef775033524e",
"name": "GitHub",
"dockerRepository": "airbyte/source-github",
"dockerImageTag": "0.2.15",
"dockerImageTag": "0.2.16",
"documentationUrl": "https://docs.airbyte.io/integrations/sources/github",
"icon": "github.svg"
}
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@
- name: GitHub
sourceDefinitionId: ef69ef6e-aa7f-4af1-a01d-ef775033524e
dockerRepository: airbyte/source-github
dockerImageTag: 0.2.15
dockerImageTag: 0.2.16
documentationUrl: https://docs.airbyte.io/integrations/sources/github
icon: github.svg
sourceType: api
Expand Down
14 changes: 8 additions & 6 deletions airbyte-config/init/src/main/resources/seed/source_specs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2113,7 +2113,7 @@
supportsNormalization: false
supportsDBT: false
supported_destination_sync_modes: []
- dockerImage: "airbyte/source-github:0.2.15"
- dockerImage: "airbyte/source-github:0.2.16"
spec:
documentationUrl: "https://docs.airbyte.io/integrations/sources/github"
connectionSpecification:
Expand Down Expand Up @@ -2166,12 +2166,14 @@
repository:
type: "string"
examples:
- "airbytehq/airbyte"
- "airbytehq/airbyte airbytehq/another-repo"
- "airbytehq/*"
- "airbytehq/airbyte"
title: "GitHub Repositories"
description: "Space-delimited list of GitHub repositories/organizations,\
\ e.g. `airbytehq/airbyte` for single repository and `airbytehq/*` for\
\ get all repositories from organization"
description: "Space-delimited list of GitHub organizations/repositories,\
\ e.g. `airbytehq/airbyte` for single repository, `airbytehq/*` for get\
\ all repositories from organization and `airbytehq/airbyte airbytehq/another-repo`\
\ for multiple repositories."
start_date:
type: "string"
title: "Start Date"
Expand All @@ -2186,7 +2188,7 @@
type: "string"
title: "Branch"
examples:
- "airbytehq/airbyte/master"
- "airbytehq/airbyte/master airbytehq/airbyte/my-branch"
description: "Space-delimited list of GitHub repository branches to pull\
\ commits for, e.g. `airbytehq/airbyte/master`. If no branches are specified\
\ for a repository, the default branch will be pulled."
Expand Down
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/source-github/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ RUN pip install .
ENV AIRBYTE_ENTRYPOINT "python /airbyte/integration_code/main.py"
ENTRYPOINT ["python", "/airbyte/integration_code/main.py"]

LABEL io.airbyte.version=0.2.15
LABEL io.airbyte.version=0.2.16
LABEL io.airbyte.name=airbyte/source-github
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ tests:
connection:
- config_path: "secrets/config.json"
status: "succeed"
- config_path: "secrets/config_oauth.json"
status: "succeed"
- config_path: "integration_tests/invalid_config.json"
status: "failed"
discovery:
- config_path: "secrets/config.json"
- config_path: "secrets/config_oauth.json"
basic_read:
- config_path: "secrets/config.json"
configured_catalog_path: "integration_tests/configured_catalog.json"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,23 +81,33 @@ def backoff_time(self, response: requests.Response) -> Union[int, float]:
return max(backoff_time, 60) # This is a guarantee that no negative value will be returned.

def read_records(self, stream_slice: Mapping[str, any] = None, **kwargs) -> Iterable[Mapping[str, Any]]:
# get out the stream_slice parts for later use.
organisation = stream_slice.get("organization", "")
repository = stream_slice.get("repository", "")
# Reading records while handling the errors
try:
yield from super().read_records(stream_slice=stream_slice, **kwargs)
except HTTPError as e:
error_msg = str(e)

error_msg = str(e.response.json().get("message"))
# This whole try/except situation in `read_records()` isn't good but right now in `self._send_request()`
# function we have `response.raise_for_status()` so we don't have much choice on how to handle errors.
# Bocked on https://github.com/airbytehq/airbyte/issues/3514.
if e.response.status_code == requests.codes.FORBIDDEN:
# When using the `check` method, we should raise an error if we do not have access to the repository.
# When using the `check_connection` method, we should raise an error if we do not have access to the repository.
if isinstance(self, Repositories):
raise e
error_msg = (
f"Syncing `{self.name}` stream isn't available for repository "
f"`{stream_slice['repository']}` and your `access_token`, seems like you don't have permissions for "
f"this stream."
)
# When `403` for the stream, that has no access to the organization's teams, based on OAuth Apps Restrictions:
# https://docs.github.com/en/organizations/restricting-access-to-your-organizations-data/enabling-oauth-app-access-restrictions-for-your-organization
# For all `Organisation` based streams
elif isinstance(self, Organizations) or isinstance(self, Teams) or isinstance(self, Users):
error_msg = (
f"Syncing `{self.name}` stream isn't available for organization `{organisation}`. Full error message: {error_msg}"
)
# For all other `Repository` base streams
else:
error_msg = (
f"Syncing `{self.name}` stream isn't available for repository `{repository}`. Full error message: {error_msg}"
)
elif e.response.status_code == requests.codes.NOT_FOUND and "/teams?" in error_msg:
# For private repositories `Teams` stream is not available and we get "404 Client Error: Not Found for
# url: https://api.github.com/orgs/<org_name>/teams?per_page=100" error.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import java.io.IOException;
import java.net.URISyntaxException;
import java.net.http.HttpClient;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.function.Supplier;
Expand All @@ -30,7 +32,18 @@ public class GithubOAuthFlow extends BaseOAuth2Flow {
// tune up.
// This is necessary to pull data from private repositories.
// https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps#available-scopes
private static final String SCOPES = "repo";

private static final List<String> SCOPES = Arrays.asList(
"repo",
"read:org",
"read:repo_hook",
"read:user",
"read:discussion",
"workflow");

public String getScopes() {
return String.join("%20", SCOPES);
}

public GithubOAuthFlow(final ConfigRepository configRepository, final HttpClient httpClient) {
super(configRepository, httpClient);
Expand All @@ -47,13 +60,15 @@ protected String formatConsentUrl(final UUID definitionId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration)
throws IOException {

try {
return new URIBuilder(AUTHORIZE_URL)
.addParameter("client_id", clientId)
.addParameter("redirect_uri", redirectUrl)
.addParameter("scope", SCOPES)
.addParameter("state", getState())
.build().toString();
// we add `scopes` and `state` after we've already built the url, to prevent url encoding for scopes
// https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps#available-scopes
// we need to keep scopes in the format of: < scope1%20scope2:sub_scope%20scope3 >
.build().toString() + "&scope=" + getScopes() + "&state=" + getState();
} catch (final URISyntaxException e) {
throw new IOException("Failed to format Consent URL for OAuth flow", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ protected BaseOAuthFlow getOAuthFlow() {

@Override
protected String getExpectedConsentUrl() {
return "https://github.com/login/oauth/authorize?client_id=test_client_id&redirect_uri=https%3A%2F%2Fairbyte.io&scope=repo&state=state";
return "https://github.com/login/oauth/authorize?client_id=test_client_id&redirect_uri=https%3A%2F%2Fairbyte.io&scope=repo%20read:org%20read:repo_hook%20read:user%20read:discussion%20workflow&state=state";
}

@Override
Expand Down