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 Google Analytics Data API: implement advanced reports #25179

Conversation

davydov-d
Copy link
Collaborator

What

#11689

How

Implement support for Cohort reports and Pivot reports according to https://developers.google.com/analytics/devguides/reporting/data/v1

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/google-analytics-data-api labels Apr 13, 2023
@davydov-d
Copy link
Collaborator Author

davydov-d commented Apr 13, 2023

/test connector=connectors/source-google-analytics-data-api

🕑 connectors/source-google-analytics-data-api https://github.com/airbytehq/airbyte/actions/runs/4692406047

@davydov-d
Copy link
Collaborator Author

davydov-d commented Apr 13, 2023

/test connector=connectors/source-google-analytics-data-api

🕑 connectors/source-google-analytics-data-api https://github.com/airbytehq/airbyte/actions/runs/4692450643
❌ connectors/source-google-analytics-data-api https://github.com/airbytehq/airbyte/actions/runs/4692450643
🐛 https://gradle.com/s/sbcbubhwyodqq

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Stream traff...
FAILED test_incremental.py::TestIncremental::test_two_sequential_reads[inputs0]
FAILED test_incremental.py::TestIncremental::test_read_sequential_slices[inputs0]
FAILED test_incremental.py::TestIncremental::test_state_with_abnormally_large_values[inputs0]
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.
====== 4 failed, 35 passed, 1 skipped, 102 warnings in 3683.96s (1:01:23) ======

> Task :airbyte-integrations:connectors:source-google-analytics-data-api:connectorAcceptanceTest FAILED

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
27 actionable tasks: 9 executed, 18 up-to-date

Publishing build scan...
https://gradle.com/s/sbcbubhwyodqq

S3 cache writes: 1, elapsed: 685ms, sent to cache: 455 B

@davydov-d
Copy link
Collaborator Author

davydov-d commented Apr 14, 2023

/test connector=connectors/source-google-analytics-data-api

🕑 connectors/source-google-analytics-data-api https://github.com/airbytehq/airbyte/actions/runs/4697044496

@davydov-d
Copy link
Collaborator Author

davydov-d commented Apr 14, 2023

/test connector=connectors/source-google-analytics-data-api

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

Name                                                Stmts   Miss  Cover
-----------------------------------------------------------------------
source_google_analytics_data_api/__init__.py            2      0   100%
source_google_analytics_data_api/authenticator.py      44      2    95%
source_google_analytics_data_api/utils.py              26      2    92%
source_google_analytics_data_api/source.py            225     28    88%
source_google_analytics_data_api/api_quota.py          94     12    87%
-----------------------------------------------------------------------
TOTAL                                                 391     44    89%

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:578: The previous and actual discovered catalogs are identical.
=========== 39 passed, 1 skipped, 102 warnings in 202.85s (0:03:22) ============

@davydov-d davydov-d requested a review from a team April 14, 2023 08:00
next_page_token: Mapping[str, Any] = None,
) -> Optional[Mapping]:
# https://developers.google.com/analytics/devguides/reporting/data/v1/rest/v1beta/CohortSpec#Cohort.FIELDS.date_range
# In a cohort request, this dateRange is required and the dateRanges in the RunReportRequest or RunPivotReportRequest
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why you delete dateRange from payload if you said that is required for cohort request. And this mixin is parent only for cohort report.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are different date ranges. The one in the root of the payload should be unspecified because there is another one inside the "cohortSpec" object (see schema for details).

This mixin is one of the base classes for the dynamically created report if there's a "cohortSpec" object in the report definition. It can be either a regular report or a pivot report.

@davydov-d davydov-d requested a review from lazebnyi April 14, 2023 16:35
@lazebnyi lazebnyi requested a review from clnoll April 18, 2023 14:11
Copy link
Contributor

@clnoll clnoll left a comment

Choose a reason for hiding this comment

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

Thanks @davydov-d! Just a few questions for you.

@arsenlosenko
Copy link
Contributor

Hi @clnoll, @davydov-d is currently on PTO, so I'll dive into this PR and answer your questions regarding it a bit later.

@arsenlosenko
Copy link
Contributor

arsenlosenko commented Apr 24, 2023

/test connector=connectors/source-google-analytics-data-api

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

Name                                                Stmts   Miss  Cover
-----------------------------------------------------------------------
source_google_analytics_data_api/__init__.py            2      0   100%
source_google_analytics_data_api/authenticator.py      44      2    95%
source_google_analytics_data_api/utils.py              26      2    92%
source_google_analytics_data_api/source.py            225     28    88%
source_google_analytics_data_api/api_quota.py          94     12    87%
-----------------------------------------------------------------------
TOTAL                                                 391     44    89%

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:578: The previous and actual discovered catalogs are identical.
=========== 39 passed, 1 skipped, 102 warnings in 200.28s (0:03:20) ============

Copy link
Contributor

@clnoll clnoll left a comment

Choose a reason for hiding this comment

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

LGTM!

@arsenlosenko arsenlosenko removed the request for review from a team April 25, 2023 13:45
@arsenlosenko
Copy link
Contributor

arsenlosenko commented Apr 25, 2023

/publish connector=connectors/source-google-analytics-data-api

🕑 Publishing the following connectors:
connectors/source-google-analytics-data-api
https://github.com/airbytehq/airbyte/actions/runs/4799276846


Connector Version Did it publish? Were definitions generated?
connectors/source-google-analytics-data-api 0.2.0

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

@arsenlosenko arsenlosenko merged commit 3f327f6 into master Apr 25, 2023
@arsenlosenko arsenlosenko deleted the ddavydov/#11689-source-google-analytics-implement-advanced-reports branch April 25, 2023 16:55
marcosmarxm pushed a commit to natalia-miinto/airbyte that referenced this pull request Jun 8, 2023
…hq#25179)

* airbytehq#11689 source Google Analytics Data API: implement advanced reports

* airbytehq#11689 source GA data API: upd changelog

* airbytehq#11689 source GA data API: fix CAT

* airbytehq#11689 source GA data API: fix CAT

* Have empty dict as default option

* Remove dateRange and dimensions from required params

* Generate source defintions manually

---------

Co-authored-by: Arsen Losenko <20901439+arsenlosenko@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/google-analytics-data-api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants