-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Fix check connection for earlier date in Google Analytics #8087
Fix check connection for earlier date in Google Analytics #8087
Conversation
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.
Instead of guessing which slice contains records you could just modify request to specify whole date range from start_date up to now and set pageSize parameter to 1 (instead of 100000). Please read docs for details
@keu @sergei-solonitcyn @avida 1st failed test.
2nd failed test
|
but this could be a lot of data and has a high chance of timing out... |
airbyte-integrations/connectors/source-google-analytics-v4/source_google_analytics_v4/source.py
Show resolved
Hide resolved
…r-earlier-date-ga' into arymkhan/fix-check-connection-for-earlier-date-ga
We set page_size = 1 in TestStreamConnection |
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.
lgtm
airbyte-integrations/connectors/source-google-analytics-v4/source_google_analytics_v4/source.py
Outdated
Show resolved
Hide resolved
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.
Nicely done with adding the extra unit tests!
airbyte-integrations/connectors/source-google-analytics-v4/source_google_analytics_v4/source.py
Outdated
Show resolved
Hide resolved
/publish connector=connectors/source-google-analytics-v4
|
/publish connector=connectors/source-google-analytics-v4
|
/publish connector=connectors/source-google-analytics-v4 --run-tests=false
|
/publish connector=connectors/source-google-analytics-v4 run-tests=false
|
…8087) * Changed tests for `check_connection` method * Start reading from the latest date-slice to check the reading permissions in `check_connection` * Fetch records from start_date up to now for testing case * Add minimal valid records into response_with_records.json * Changed version of the connector * formatted codes * Override stream_slices in Test stream * return list of date slices inline * replace valiation msg to "Cannot retrieve data from that view ID" * formatted code * fixed error msg in unit tests * changed version in seed/ yaml files Co-authored-by: Auganbay <auganenu@gmail.com>
What
This error happens If a user enters
start_date
before the date when Google analytics account had the first data.In
check_connection
method the current implementation tries to get data for the first date-slice to check the reading permissions.For example:
start_date
, andwindows_in_date = 30
3.
check_connection
method sends a request only for the first slice (2021-01-01 - 2021-01-31)read_check list is empty because there is no data for that slice. Also there no exception is raised, so
check_connection
methods returnsNone
self.check could not unpack
None
How
In
check_connection
read records using test streamTestStreamConnection
, which haspag_size = 1
and overridenrequest_body_json
method where we configure request to fetch records fromstart_date
up to now for testing case.If it returns some valid records, we assume a user has the right permission to read, otherwise we return validation error message.
Recommended reading order
x.java
y.python
🚨 User Impact 🚨
Are there any breaking changes? If yes, please make sure to include it here and in any changelogs with the 🚨🚨 emoji
What is the end result perceived by the user?
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/SUMMARY.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
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./publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including 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./publish
command described hereConnector Generator
-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