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.

Sign up for GitHub

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

Jump to bottom

Google analytics v4: Oauth2.0 config #6306

Merged
merged 2 commits into from
Sep 21, 2021
Merged

Google analytics v4: Oauth2.0 config #6306

merged 2 commits into from
Sep 21, 2021

Conversation

avida
Copy link
Contributor

@avida avida commented Sep 20, 2021 •
edited
Loading

What

Describe what the change is solving
It helps to add screenshots if it affects the frontend.

How

Resolves #6173

Recommended reading order

  1. x.java
  2. y.python

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

  • Community member? 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
    • docs/SUMMARY.md
    • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
    • docs/integrations/README.md
    • airbyte-integrations/builds.md
  • PR name follows PR naming conventions
  • Connector added to connector index like described here

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
  • Credentials added to Github CI. Instructions.
  • /test connector=connectors/<name> command is passing.
  • New Connector version released on Dockerhub by running the /publish command described here

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
  • Connector version bumped like described here

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
  • Credentials added to Github CI. Instructions.
  • /test connector=connectors/<name> command is passing.
  • New Connector version released on Dockerhub by running the /publish command described here

Connector Generator

  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • If adding a new generator, add it to the list of scaffold modules being tested
  • The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
  • Documentation which references the generator is updated as needed.

@github-actions github-actions bot added the area/connectors Connector related issues label Sep 20, 2021
@avida avida force-pushed the drezchykov/google-analytics-v4-oauth branch from fbf77be to 5628ef8 Compare September 20, 2021 09:36
@avida avida temporarily deployed to more-secrets September 20, 2021 09:37 Inactive
@avida avida force-pushed the drezchykov/google-analytics-v4-oauth branch from 5628ef8 to fe20a1d Compare September 20, 2021 11:10
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Sep 20, 2021
@avida avida temporarily deployed to more-secrets September 20, 2021 11:12 Inactive
@avida avida force-pushed the drezchykov/google-analytics-v4-oauth branch 2 times, most recently from 98f2706 to c1d3af5 Compare September 20, 2021 11:53
@avida
Copy link
Contributor Author

avida commented Sep 20, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-google-analytics-v4

🕑 connectors/source-google-analytics-v4 https://github.com/airbytehq/airbyte/actions/runs/1253404701
✅ connectors/source-google-analytics-v4 https://github.com/airbytehq/airbyte/actions/runs/1253404701
Python tests coverage:

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                 Stmts   Miss  Cover
	 ------------------------------------------------------------------------
	 source_acceptance_test/__init__.py                       2      0   100%
	 source_acceptance_test/base.py                          10      4    60%
	 source_acceptance_test/config.py                        74      8    89%
	 source_acceptance_test/conftest.py                     108    108     0%
	 source_acceptance_test/plugin.py                        45     45     0%
	 source_acceptance_test/tests/__init__.py                 4      0   100%
	 source_acceptance_test/tests/test_core.py              158    109    31%
	 source_acceptance_test/tests/test_full_refresh.py       18     11    39%
	 source_acceptance_test/tests/test_incremental.py        69     38    45%
	 source_acceptance_test/utils/__init__.py                 6      0   100%
	 source_acceptance_test/utils/asserts.py                 37      2    95%
	 source_acceptance_test/utils/common.py                  41     25    39%
	 source_acceptance_test/utils/compare.py                 47     20    57%
	 source_acceptance_test/utils/connector_runner.py        82     49    40%
	 source_acceptance_test/utils/json_schema_helper.py      75     11    85%
	 ------------------------------------------------------------------------
	 TOTAL                                                  776    430    45%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                     Stmts   Miss  Cover
	 ------------------------------------------------------------
	 source_google_analytics_v4/__init__.py       2      0   100%
	 source_google_analytics_v4/source.py       225    106    53%
	 ------------------------------------------------------------
	 TOTAL                                      227    106    53%

@avida avida temporarily deployed to more-secrets September 20, 2021 11:54 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 20, 2021 11:56 Inactive
@avida avida force-pushed the drezchykov/google-analytics-v4-oauth branch from c1d3af5 to 70a6791 Compare September 20, 2021 13:06
@avida avida changed the title WIP: Google analytics v4: Oauth2.0 config Google analytics v4: Oauth2.0 config Sep 20, 2021
@avida avida temporarily deployed to more-secrets September 20, 2021 13:07 Inactive
@avida avida force-pushed the drezchykov/google-analytics-v4-oauth branch from 70a6791 to 32a3067 Compare September 20, 2021 14:37
@avida avida temporarily deployed to more-secrets September 20, 2021 14:39 Inactive
@ChristopheDuong ChristopheDuong mentioned this pull request Sep 20, 2021
Merged
sherifnada
sherifnada approved these changes Sep 20, 2021
View reviewed changes
airbyte-integrations/connectors/source-google-analytics-v4/source_google_analytics_v4/spec.json Outdated
"title": "Credentials JSON",
"description": "The contents of the JSON service account key. Check out the <a href=\"https://docs.airbyte.io/integrations/sources/googleanalytics\">docs</a> if you need help generating this key.",
"airbyte_secret": true
"auth_mechanism": {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you name this credentials so we can have consistency across connectors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

airbyte-integrations/connectors/source-google-analytics-v4/source_google_analytics_v4/spec.json Outdated
@@ -4,14 +4,48 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "Google Analytics V4 Spec",
"type": "object",
"required": ["credentials_json", "view_id", "start_date"],
"required": ["view_id", "start_date"],
Copy link
Contributor

Choose a reason for hiding this comment

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

credentials is required

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

airbyte-integrations/connectors/source-google-analytics-v4/source_google_analytics_v4/spec.json
"title": "Authentication mechanism",
"type": "object",
"description": "Choose either OAuth2.0 flow or provide your own JWT credentials for service account",
"oneOf": [
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to make this backwards compatible so it's not a break change? Maybe additionalProps=true and if credentials_json is available in the config then continue functioning correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

airbyte-integrations/connectors/source-google-analytics-v4/source_google_analytics_v4/spec.json Outdated
},
{
"type": "object",
"title": "JWT authorization",
Copy link
Contributor

Choose a reason for hiding this comment

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

We can just call this Service Account Key everywhere instead of JWT. I think that terminology is more common.l

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@avida
Copy link
Contributor Author

avida commented Sep 21, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-google-analytics-v4

🕑 connectors/source-google-analytics-v4 https://github.com/airbytehq/airbyte/actions/runs/1256598338
✅ connectors/source-google-analytics-v4 https://github.com/airbytehq/airbyte/actions/runs/1256598338
Python tests coverage:

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                 Stmts   Miss  Cover
	 ------------------------------------------------------------------------
	 source_acceptance_test/__init__.py                       2      0   100%
	 source_acceptance_test/base.py                          10      4    60%
	 source_acceptance_test/config.py                        74      8    89%
	 source_acceptance_test/conftest.py                     108    108     0%
	 source_acceptance_test/plugin.py                        45     45     0%
	 source_acceptance_test/tests/__init__.py                 4      0   100%
	 source_acceptance_test/tests/test_core.py              158    109    31%
	 source_acceptance_test/tests/test_full_refresh.py       18     11    39%
	 source_acceptance_test/tests/test_incremental.py        69     38    45%
	 source_acceptance_test/utils/__init__.py                 6      0   100%
	 source_acceptance_test/utils/asserts.py                 37      2    95%
	 source_acceptance_test/utils/common.py                  41     25    39%
	 source_acceptance_test/utils/compare.py                 47     20    57%
	 source_acceptance_test/utils/connector_runner.py        82     49    40%
	 source_acceptance_test/utils/json_schema_helper.py      75     11    85%
	 ------------------------------------------------------------------------
	 TOTAL                                                  776    430    45%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                     Stmts   Miss  Cover
	 ------------------------------------------------------------
	 source_google_analytics_v4/__init__.py       2      0   100%
	 source_google_analytics_v4/source.py       227    107    53%
	 ------------------------------------------------------------
	 TOTAL                                      229    107    53%

@avida avida temporarily deployed to more-secrets September 21, 2021 07:16 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 21, 2021 07:17 Inactive
@avida
Copy link
Contributor Author

avida commented Sep 21, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-google-analytics-v4

🕑 connectors/source-google-analytics-v4 https://github.com/airbytehq/airbyte/actions/runs/1256661949
✅ connectors/source-google-analytics-v4 https://github.com/airbytehq/airbyte/actions/runs/1256661949

@jrhizor jrhizor temporarily deployed to more-secrets September 21, 2021 07:39 Inactive
@avida avida merged commit 52c4eaa into master Sep 21, 2021
@avida avida deleted the drezchykov/google-analytics-v4-oauth branch September 21, 2021 07:49
@avida avida mentioned this pull request Sep 24, 2021
Merged
15 tasks
@jrhizor jrhizor mentioned this pull request Sep 25, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zirochkaa Zirochkaa Zirochkaa approved these changes

@sherifnada sherifnada sherifnada approved these changes

@htrueman htrueman Awaiting requested review from htrueman

@yevhenii-ldv yevhenii-ldv Awaiting requested review from yevhenii-ldv

@ChristopheDuong ChristopheDuong Awaiting requested review from ChristopheDuong

Assignees
No one assigned
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Source Google Analytics: Support webflow Oauth
5 participants
@avida @sherifnada @ChristopheDuong @Zirochkaa @jrhizor