🎉 Source Google Analytics v4: Declare oauth parameters in google sources #6414
Conversation
| @@ -4,49 +4,9 @@ | |||
| "$schema": "http://json-schema.org/draft-07/schema#", | |||
| "title": "Google Analytics V4 Spec", | |||
| "type": "object", | |||
| "required": ["credentials", "view_id", "start_date"], | |||
| "additionalProperties": true, | |||
| "required": ["authorization", "view_id", "start_date"], | |||
There was a problem hiding this comment.
we should keep the name the same since other code in the platform depends on this
| "type": "object", | ||
| "oneOf": [ | ||
| { | ||
| "title": "Client Auth", |
There was a problem hiding this comment.
could you use the same titles and descriptions in Google Search Console?
| } | ||
| } | ||
| }, | ||
| "authSpecification": { |
There was a problem hiding this comment.
this should remain, why remove it?
|
@sherifnada Google analytics oauth workflow task already completed by #6306 PR, why we need to do it one more time? |
|
|
| @@ -469,17 +442,34 @@ def get_refresh_request_params(self) -> Mapping[str, any]: | |||
| } | |||
| headers = {"kid": self.client_id} | |||
| signed_jwt = jwt.encode(payload, self.client_secret, headers=headers, algorithm="RS256") | |||
| return {"grant_type": "urn:ietf:params:oauth:grant-type:jwt-bearer", "assertion": signed_jwt} | |||
| return {"grant_type": "urn:ietf:params:oauth:grant-type:jwt-bearer", "assertion": str(signed_jwt)} | |||
There was a problem hiding this comment.
Since line 432 was removed, it doesn't seem to be using the method from super() in non-jwt cases... (ie when not using service account json?)
In those cases, it seems the grant_type should be simply a refresh_token value:
Is this making the connector supporting only the service account / jwt?
There was a problem hiding this comment.
The get_authenticator method contains the logic for choosing the authorization type, there is support for client and service authorization
|
/test connector=connectors/source-google-analytics-v4
|
|
|
||
|
|
||
| class SourceGoogleAnalyticsV4(AbstractSource): | ||
| """Google Analytics lets you analyze data about customer engagement with your website or application.""" | ||
|
|
||
| @staticmethod | ||
| def get_authenticator(config): | ||
| if config.get("credentials_json"): |
There was a problem hiding this comment.
| if config.get("credentials_json"): | |
| # backwards compatibility, credentials_json used to be in the top level of the connector | |
| if config.get("credentials_json"): |
| @@ -4,50 +4,9 @@ | |||
| "$schema": "http://json-schema.org/draft-07/schema#", | |||
| "title": "Google Analytics V4 Spec", | |||
| "type": "object", | |||
| "required": ["credentials", "view_id", "start_date"], | |||
| "required": ["view_id", "start_date"], | |||
There was a problem hiding this comment.
why remove credentials as required?
There was a problem hiding this comment.
the old configuration does not contain a credential field and when we run the connector the following error appears Exception: Config validation error: 'credentials' is a required property
There was a problem hiding this comment.
ah I see, good point. Thanks for the clarification
| @@ -81,14 +84,14 @@ def test_check_connection_oauth(jwt_encode_mock, mocker, mock_metrics_dimensions | |||
| test_config = json.loads(read_file("../integration_tests/sample_config.json")) | |||
| del test_config["custom_reports"] | |||
| test_config["credentials"] = { | |||
| "auth_type": "Client", | |||
| "client_id": "client_id_val", | |||
| "client_secret": "client_secret_val", | |||
| "refresh_token": "refresh_token_val", | |||
| } | |||
| source = SourceGoogleAnalyticsV4() | |||
| assert source.check_connection(MagicMock(), test_config) == (True, None) | |||
| jwt_encode_mock.encode.assert_not_called() | |||
|
/test connector=connectors/source-google-analytics-v4
|
|
/test connector=connectors/source-google-analytics-v4
|
github-actions
bot
added
the
area/documentation
|
/test connector=connectors/source-google-analytics-v4
|
|
/publish connector=connectors/source-google-analytics-v4
|
|
I created a bug mostly related to this PR, please see #6977 |
…ces (airbytehq#6414) * Upd auth: oauth support * Rename authorization/credentials, upd spec, refactor * Add backward compatibility * Upd CI * Bump version * Upd changelog
What
Add support for connecting via Ouath webflow
How
Update spec.json schema to support 2 types of authorization
Update source.py
Update unit_tests
Pre-merge Checklist
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing./publishcommand described here