-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 Amazon Ads: get rid of fail_on_extra_columns: false
in SAT
#25913
Source Amazon Ads: get rid of fail_on_extra_columns: false
in SAT
#25913
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,
|
/test connector=connectors/source-amazon-ads
Build FailedTest summary info:
|
/test connector=connectors/source-amazon-ads
Build PassedTest summary info:
|
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! Minor changes needed.
@@ -36,6 +36,9 @@ def schema_extra(cls, schema: Dict[str, Any], model: Type["BaseModel"]) -> None: | |||
if "type" in prop: | |||
if allow_none: | |||
prop["type"] = ["null", prop["type"]] | |||
if prop["type"] == "array" and prop["items"]: |
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.
let's add unit tests to check this functionality.
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.
done
@roman-yermilov-gl can you please lay out what changes are made and why, in the PR description? Will remove my request until then so it doesn't show up in my list, but feel free to rerequest |
fail_on_extra_columns
to false
fail_on_extra_columns
to false
fail_on_extra_columns: false
in SAT
/test connector=connectors/source-amazon-ads
Build PassedTest summary info:
|
/publish connector=connectors/source-amazon-ads
if you have connectors that successfully published but failed definition generation, follow step 4 here |
1fca5bf
to
0448776
Compare
/publish connector=connectors/source-amazon-ads
if you have connectors that successfully published but failed definition generation, follow step 4 here |
0448776
to
3bb4548
Compare
/publish connector=connectors/source-amazon-ads
if you have connectors that successfully published but failed definition generation, follow step 4 here |
…25913) * Source Amazon Ads: small schema fixes * Source Amazon Ads: update changelog * Source Amazon Ads: update unittest * Source Amazon Ads: unittest additional property is boolean * Source Amazon Ads: bump version * auto-bump connector version --------- Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
* Remove Definition File References from Python files (#25590) * Remove check_images_exist.sh * Update definitions.py * Update build_report.py * Update tools/bin/ci_integration_workflow_launcher.py * Update tools/bin/ci_check_dependency.py * tools/bin/scan_all_spec_airbyte_secret.py * Remove qa engine references * Revert "Remove check_images_exist.sh" This reverts commit 7675162. * Improve get url function * Add test * remove scan_all_spec_airbyte_secret.py * add additional test * Remove check_images_exist.sh (#25593) * Remove Definition File References from Java files (LocalDefinitionProvider), shell scripts and docs (#25592) * Remove CombinedConnectorCatalogGenerator.java * Update local definition provider * Update local def test * Add spec mask downloader * Make downloader work * Delete generators and add tests * REMOVE THE YAML FILES * Roughly update docs * Update shell scripts * Remove unused * Add connector metadata file doc * Apply suggestions from code review Co-authored-by: Augustin <augustin@airbyte.io> * Additional PR comments * Run format tasks --------- Co-authored-by: Augustin <augustin@airbyte.io> * Remove unused import * bundle registry * Ignore future updates * Update registry * new registry * Readd maskeddatainterceptor * Automated Change * Remove icon validation * Automated Change * Automated Change * Source Amazon Ads: get rid of `fail_on_extra_columns: false` in SAT (#25913) * Source Amazon Ads: small schema fixes * Source Amazon Ads: update changelog * Source Amazon Ads: update unittest * Source Amazon Ads: unittest additional property is boolean * Source Amazon Ads: bump version * auto-bump connector version --------- Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com> * connectors-ci: make spec-cache / metadata bucket and creds not required for pre-release (#26119) * Automated Change --------- Co-authored-by: Augustin <augustin@airbyte.io> Co-authored-by: bnchrch <bnchrch@users.noreply.github.com> Co-authored-by: Roman Yermilov [GL] <86300758+roman-yermilov-gl@users.noreply.github.com> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
…irbytehq#25913) * Source Amazon Ads: small schema fixes * Source Amazon Ads: update changelog * Source Amazon Ads: update unittest * Source Amazon Ads: unittest additional property is boolean * Source Amazon Ads: bump version * auto-bump connector version --------- Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
* Remove Definition File References from Python files (airbytehq#25590) * Remove check_images_exist.sh * Update definitions.py * Update build_report.py * Update tools/bin/ci_integration_workflow_launcher.py * Update tools/bin/ci_check_dependency.py * tools/bin/scan_all_spec_airbyte_secret.py * Remove qa engine references * Revert "Remove check_images_exist.sh" This reverts commit 7675162. * Improve get url function * Add test * remove scan_all_spec_airbyte_secret.py * add additional test * Remove check_images_exist.sh (airbytehq#25593) * Remove Definition File References from Java files (LocalDefinitionProvider), shell scripts and docs (airbytehq#25592) * Remove CombinedConnectorCatalogGenerator.java * Update local definition provider * Update local def test * Add spec mask downloader * Make downloader work * Delete generators and add tests * REMOVE THE YAML FILES * Roughly update docs * Update shell scripts * Remove unused * Add connector metadata file doc * Apply suggestions from code review Co-authored-by: Augustin <augustin@airbyte.io> * Additional PR comments * Run format tasks --------- Co-authored-by: Augustin <augustin@airbyte.io> * Remove unused import * bundle registry * Ignore future updates * Update registry * new registry * Readd maskeddatainterceptor * Automated Change * Remove icon validation * Automated Change * Automated Change * Source Amazon Ads: get rid of `fail_on_extra_columns: false` in SAT (airbytehq#25913) * Source Amazon Ads: small schema fixes * Source Amazon Ads: update changelog * Source Amazon Ads: update unittest * Source Amazon Ads: unittest additional property is boolean * Source Amazon Ads: bump version * auto-bump connector version --------- Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com> * connectors-ci: make spec-cache / metadata bucket and creds not required for pre-release (airbytehq#26119) * Automated Change --------- Co-authored-by: Augustin <augustin@airbyte.io> Co-authored-by: bnchrch <bnchrch@users.noreply.github.com> Co-authored-by: Roman Yermilov [GL] <86300758+roman-yermilov-gl@users.noreply.github.com> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
What
Amazon Ads passes CAT without declaring
fail_on_extra_columns: false
How
The following descriptions of streams that pass undeclared columns come from results of the failed connector acceptance test:
Additional properties are not allowed ('<column>', 'column' were unexpected)
logs to the connector's spec.fail_on_extra_columns: false
from theacceptance-test-config.yml
file.acceptance-test-config.yml
and open a PR./test
command