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.

Sign up for GitHub

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

Jump to bottom

GoogleAds Connector multiple customer_ids, fix #3154

Merged
merged 9 commits into from
May 20, 2021
Merged

GoogleAds Connector multiple customer_ids, fix #3154

merged 9 commits into from
May 20, 2021

Conversation

po3na4skld
Copy link
Contributor

What

Fixing this issue: #2903

How

New pattern in spec.json was added in customers_ids field

Pre-merge Checklist

  • Run integration tests
  • Publish Docker images

Recommended reading order

  1. spec.json

@Zirochkaa Zirochkaa linked an issue Apr 30, 2021 that may be closed by this pull request
Google Ads Connector says it supports multiple customer ids but only accepts one. #2903
Closed
@Zirochkaa Zirochkaa requested review from vitaliizazmic, Zirochkaa, keu and yevhenii-ldv and removed request for michel-tricot and davinchia April 30, 2021 14:45
michel-tricot
michel-tricot reviewed May 1, 2021
View reviewed changes
...-integrations/connectors/source-google-adwords-singer/source_google_adwords_singer/spec.json Outdated
@@ -47,7 +47,7 @@
},
"customer_ids": {
"type": "string",
"pattern": "^[0-9]{10}$",
"pattern": "^[0-9]{10}(,[0-9]{10})*$",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use actual lists instead? The UI now supports it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I changed it to lists

@keu
Copy link
Contributor

keu commented May 11, 2021

@po3na4skld you should run ./gradlew --no-daemon format --scan and commit all changed files.

@po3na4skld
Copy link
Contributor Author

@po3na4skld you should run ./gradlew --no-daemon format --scan and commit all changed files.

What is the purpose of this? Just wondering

@po3na4skld
Copy link
Contributor Author

@po3na4skld you should run ./gradlew --no-daemon format --scan and commit all changed files.
I'm getting this after command had run:
nothing to commit, working tree clean

@keu
Copy link
Contributor

keu commented May 12, 2021 •
edited

the pipeline is failing, https://github.com/airbytehq/airbyte/pull/3154/checks?check_run_id=2517126634
this command
./gradlew --no-daemon format --scan
pipeline executes to format your code

@po3na4skld
gradlew changes
843c163
@po3na4skld
Merge branch 'master' of github.com:airbytehq/airbyte into 2903-Googl…
f064b83 
…e-Ads-Connector-multiple-customer-ids-bugfix

to catchup master files
@keu
Copy link
Contributor

keu commented May 19, 2021

@po3na4skld please bump version and publish connector

@keu
Copy link
Contributor

keu commented May 19, 2021 •
edited by github-actions bot

/test connector=source-google-adwords-singer

🕑 source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/856689087
❌ source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/856689087

@keu
Copy link
Contributor

keu commented May 19, 2021 •
edited by github-actions bot

/test connector=source-google-adwords-singer

🕑 source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/858247148
❌ source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/858247148

@po3na4skld
Copy link
Contributor Author

po3na4skld commented May 20, 2021 •
edited by github-actions bot

/test connector=source-google-adwords-singer

🕑 source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/859956388
❌ source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/859956388

@po3na4skld
Copy link
Contributor Author

po3na4skld commented May 20, 2021 •
edited by github-actions bot

/publish connector=connectors/source-google-adwords-singer

🕑 connectors/source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/860208578
❌ connectors/source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/860208578

@po3na4skld
Copy link
Contributor Author

po3na4skld commented May 20, 2021 •
edited by github-actions bot

/test connector=source-google-adwords-singer

🕑 source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/860513671
✅ source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/860513671

@po3na4skld
Copy link
Contributor Author

po3na4skld commented May 20, 2021 •
edited by github-actions bot

/publish connector=connectors/source-google-adwords-singer

🕑 connectors/source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/860552256
✅ connectors/source-google-adwords-singer https://github.com/airbytehq/airbyte/actions/runs/860552256

@po3na4skld po3na4skld merged commit 428ef25 into master May 20, 2021
@po3na4skld po3na4skld deleted the 2903-Google-Ads-Connector-multiple-customer-ids-bugfix branch May 20, 2021 13:55
@sherifnada sherifnada mentioned this pull request May 20, 2021
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michel-tricot michel-tricot michel-tricot left review comments

@keu keu keu approved these changes

@yevhenii-ldv yevhenii-ldv yevhenii-ldv approved these changes

@vitaliizazmic vitaliizazmic Awaiting requested review from vitaliizazmic

@Zirochkaa Zirochkaa Awaiting requested review from Zirochkaa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Google Ads Connector says it supports multiple customer ids but only accepts one.
4 participants
@po3na4skld @keu @michel-tricot @yevhenii-ldv