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 Ads: Delete required start date #30071
🐛 Source Google Ads: Delete required start date #30071
Conversation
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
source-google-ads test report (commit
|
Step | Result |
---|---|
Connector package install | ✅ |
Build source-google-ads docker image for platform linux/x86_64 | ✅ |
Unit tests | ❌ |
Code format checks | ✅ |
Validate airbyte-integrations/connectors/source-google-ads/metadata.yaml | ✅ |
Connector version semver check | ✅ |
Connector version increment check | ✅ |
QA checks | ✅ |
☁️ View runs for commit in Dagger Cloud
Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command
airbyte-ci connectors --name=source-google-ads test
source-google-ads test report (commit
|
Step | Result |
---|---|
Connector package install | ✅ |
Build source-google-ads docker image for platform linux/x86_64 | ✅ |
Unit tests | ✅ |
Acceptance tests | ✅ |
Code format checks | ✅ |
Validate airbyte-integrations/connectors/source-google-ads/metadata.yaml | ✅ |
Connector version semver check | ✅ |
Connector version increment check | ✅ |
QA checks | ✅ |
☁️ View runs for commit in Dagger Cloud
Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command
airbyte-ci connectors --name=source-google-ads test
ignored_fields: | ||
accounts: | ||
- name: customer.optimization_score_weight | ||
bypass_reason: "can be updated, also it is sometimes integer, sometimes float" |
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.
can manually validate typing and update it if needed?
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.
Updated
@@ -59,7 +59,7 @@ def chunk_date_range( | |||
""" | |||
today = pendulum.today(tz=time_zone) | |||
end_date = min(pendulum.parse(end_date, tz=time_zone), today) if end_date else today | |||
start_date = pendulum.parse(start_date, tz=time_zone) | |||
start_date = pendulum.parse(start_date, tz=time_zone) if start_date else pendulum.now().subtract(years=2) |
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.
If the start date is not specified, we ought to attempt to retrieve all available data. Alternatively, we can include a comment indicating that the date range query parameters are mandatory, prompting us to assign a default start date if it is absent.
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.
Date range in mandatory parameter for the query.
source-google-ads test report (commit
|
Step | Result |
---|---|
Connector package install | ✅ |
Build source-google-ads docker image for platform linux/x86_64 | ✅ |
Unit tests | ✅ |
Acceptance tests | ✅ |
Code format checks | ✅ |
Validate airbyte-integrations/connectors/source-google-ads/metadata.yaml | ✅ |
Connector version semver check | ✅ |
Connector version increment check | ✅ |
QA checks | ✅ |
☁️ View runs for commit in Dagger Cloud
Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command
airbyte-ci connectors --name=source-google-ads test
source-google-ads test report (commit
|
Step | Result |
---|---|
Connector package install | ✅ |
Build source-google-ads docker image for platform linux/x86_64 | ✅ |
Unit tests | ❌ |
Code format checks | ✅ |
Validate airbyte-integrations/connectors/source-google-ads/metadata.yaml | ✅ |
Connector version semver check | ✅ |
Connector version increment check | ✅ |
QA checks | ✅ |
☁️ View runs for commit in Dagger Cloud
Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command
airbyte-ci connectors --name=source-google-ads test
source-google-ads test report (commit
|
Step | Result |
---|---|
Connector package install | ✅ |
Build source-google-ads docker image for platform linux/x86_64 | ✅ |
Unit tests | ✅ |
Acceptance tests | ✅ |
Code format checks | ✅ |
Validate airbyte-integrations/connectors/source-google-ads/metadata.yaml | ✅ |
Connector version semver check | ✅ |
Connector version increment check | ✅ |
QA checks | ✅ |
☁️ View runs for commit in Dagger Cloud
Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command
airbyte-ci connectors --name=source-google-ads test
@@ -42,7 +42,7 @@ | |||
"type": ["null", "number"] | |||
}, | |||
"customer.optimization_score_weight": { | |||
"type": ["null", "number"] | |||
"type": ["null", "number", "integer"] |
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.
I’m not sure that we support two types for one field
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.
But we also have this type of matching in a few different connectors:
source-clockify
source-amplitude
And other
It also doesn't raise errors.
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.
I believe we can update typing during parse response.
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.
Fixed
source-google-ads test report (commit
|
Step | Result |
---|---|
Connector package install | ✅ |
Build source-google-ads docker image for platform linux/x86_64 | ✅ |
Unit tests | ✅ |
Acceptance tests | ❌ |
Code format checks | ✅ |
Validate airbyte-integrations/connectors/source-google-ads/metadata.yaml | ✅ |
Connector version semver check | ✅ |
Connector version increment check | ✅ |
QA checks | ✅ |
☁️ View runs for commit in Dagger Cloud
Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command
airbyte-ci connectors --name=source-google-ads test
source-google-ads test report (commit
|
Step | Result |
---|---|
Connector package install | ✅ |
Build source-google-ads docker image for platform linux/x86_64 | ✅ |
Unit tests | ✅ |
Acceptance tests | ❌ |
Code format checks | ✅ |
Validate airbyte-integrations/connectors/source-google-ads/metadata.yaml | ✅ |
Connector version semver check | ✅ |
Connector version increment check | ✅ |
QA checks | ✅ |
☁️ View runs for commit in Dagger Cloud
Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command
airbyte-ci connectors --name=source-google-ads test
source-google-ads test report (commit
|
Step | Result |
---|---|
Connector package install | ✅ |
Build source-google-ads docker image for platform linux/x86_64 | ✅ |
Unit tests | ✅ |
Acceptance tests | ✅ |
Code format checks | ✅ |
Validate airbyte-integrations/connectors/source-google-ads/metadata.yaml | ✅ |
Connector version semver check | ✅ |
Connector version increment check | ✅ |
QA checks | ✅ |
☁️ View runs for commit in Dagger Cloud
Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command
airbyte-ci connectors --name=source-google-ads test
source-google-ads test report (commit
|
Step | Result |
---|---|
Connector package install | ✅ |
Build source-google-ads docker image for platform linux/x86_64 | ✅ |
Unit tests | ✅ |
Acceptance tests | ✅ |
Code format checks | ✅ |
Validate airbyte-integrations/connectors/source-google-ads/metadata.yaml | ✅ |
Connector version semver check | ✅ |
Connector version increment check | ✅ |
QA checks | ✅ |
☁️ View runs for commit in Dagger Cloud
Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command
airbyte-ci connectors --name=source-google-ads test
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Hey guys. |
@oswaldobiolo Apologies for the 0.80 issues. A pagination fix is in #28970, expected to merge today. |
What
How
Set the default value for start_date as two years ago because incremental queries should include WHERE date clause.
Fix pagination to iterate over the pages of response.
Recommended reading order
start_date:
streams.py
source.py
pagination:
google_ads.py