-
Notifications
You must be signed in to change notification settings - Fork 16
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
Adding source-google-analytics-data-api #1462
base: main
Are you sure you want to change the base?
Conversation
copying the image behaviour
Remote Repo URL: [email protected]:airbytehq/airbyte.git Source name: 019153f Source Commit ID: 019153f178d221bbf602c21efea54f78042544ca Source Repo Prefix: airbyte-integrations/connectors/source-google-analytics-data-api/ Import Path: source-google-analytics-data-api/ License Type: MIT License Path: airbyte-integrations/connectors/source-google-analytics-data-api/metadata.yaml git-merge-subpath: 019153f178d221bbf602c21efea54f78042544ca airbyte-integrations/connectors/source-google-analytics-data-api source-google-analytics-data-api
The tests are broken; let me know when those are passing. |
f20dd7e
to
7d2ded0
Compare
@williamhbaker tests are now passing, deployment is breaking CI pipeline |
Can you provide a diff that compares the existing connector's outputs for discovery and spec with the final version of the one from this PR? The changes are spread between two commits here so I can't clearly tell what is changing. |
d8aa0d6
to
76df21a
Compare
Modified spec.json to match fixups
…kup/source-google-analytics-data-api
f70f4ba
to
0d92ba8
Compare
@williamhbaker Added jsondiff and capture snapshot explanation in this PR description, along with some small fixes that were occuring with a warning log on api-quota method |
The discovery diff looks good, thanks for that. There are some obvious issues with the new spec output. The documentation link is wrong, and there are missing types for some of the fields, to name a couple. Please review the spec output and make corrections so that the new connector is compatible with the old connector. |
Updated python requirements to allow for CI pipeline build
0d92ba8
to
c56a832
Compare
@williamhbaker Spec diff has been updated, Jsondiff link |
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
@williamhbaker Since now i am able to setup locally Flow Local, i wanted to run End-to-End tests on all the import PRs just to be sure. Thats why i've asked again for your review on this PR. |
@williamhbaker Just updated the PR description |
Looks great! Might be a bit overkill TBH 😄. As long as you've tested it end-to-end and documented that (a simple "manually tested by running end to end on a local stack" would be sufficient), or not tested it and documented why, that's enough - don't necessarily need the photographic proof if it saves you some time. |
@williamhbaker Got it! im going to update it in a more simple term on source-twilio and source-salesforce |
Description:
Importing source-google-analytics-data-api and making necessary changes to allow for backward compatibility with our current docker image
Notes for reviewers:
This PR was created to substitute an older PR which was created by my fork from estuary/connectors
Discover tests
Jsondiff link with discover tests. Left json is the initial snapshot, right json is the imported connector snapshot
Spec tests
jsondiff link with spec tests. Left json is the initial snapshot. Right json is the imported connector snapshot
Capture tests
Capture tests were executed on a created Google Analytics account, and was populated using a localhost HTML static page with google analytics hooks linked in the HTML header.
This Source capture test can vary daily since its based on access metrics. Because of that, capture snapshot was removed.
JSON data example
In order to compare captures in the future, one can use the following JSON object:
snapshots__capture__capture.stdout.json
End-to-End tests
Tests were made using a local flow setup and a free-tier supabase db.
Connector Shard/Spec
No issues were found while starting the connector
OAuth2
No issues were found when running OAuth2
OAuth connection:
Generated Tables
All 8 discovered tables are being generated
Data Example
Materialization example:
This change is