Skip to content

Fix dynamic imports in google ads vendored in library#30468

Closed
potiuk wants to merge 1 commit into
apache:mainfrom
potiuk:further-fix-to-googleads-vendoring
Closed

Fix dynamic imports in google ads vendored in library#30468
potiuk wants to merge 1 commit into
apache:mainfrom
potiuk:further-fix-to-googleads-vendoring

Conversation

@potiuk

@potiuk potiuk commented Apr 4, 2023

Copy link
Copy Markdown
Member

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg Bot added area:dev-tools area:providers provider:google Google (including GCP) related issues labels Apr 4, 2023
@potiuk
potiuk force-pushed the further-fix-to-googleads-vendoring branch from ad432cf to 7cd47ca Compare April 4, 2023 20:25
@potiuk
potiuk marked this pull request as ready for review April 7, 2023 17:32

@ashb ashb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Skimmed only (assuming it's automated)

@potiuk

potiuk commented Apr 7, 2023

Copy link
Copy Markdown
Member Author

We still need to get confirmation it actually works :) @collinmcnulty

@collinmcnulty

Copy link
Copy Markdown
Contributor

We have not seen this resolve the problem yet. I'm double checking that the patched code is what's running before ringing alarm bells.

@vanducng

vanducng commented Apr 8, 2023

Copy link
Copy Markdown
Contributor

Hi @potiuk @collinmcnulty, we are able to test this branch successfully for google ads api. Eager to have this provider published soon.

Test steps:

  1. Clone this forked branch

  2. Build wheel file

    breeze release-management prepare-provider-packages --version-suffix-for-pypi dev1 google
    
    image
  3. Update astro image, I need to remove the default google provider first to get it work, then install the wheel
    image

  4. Run Airflow task: succeeded!

@potiuk

potiuk commented Apr 8, 2023

Copy link
Copy Markdown
Member Author

Hi @potiuk @collinmcnulty, we are able to test this branch successfully for google ads api. Eager to have this provider published soon.

WOOOOOOHOOOOO!!!!

Let me then rebase the PR to succeed and we might want to release an ad-hoc release of Google Provider @eladkal ! Happy to do it as well if you want.

@eladkal

eladkal commented Apr 8, 2023

Copy link
Copy Markdown
Contributor

Let me then rebase the PR to succeed and we might want to release an ad-hoc release of Google Provider @eladkal ! Happy to do it as well if you want.

I will do it today

@potiuk

potiuk commented Apr 8, 2023

Copy link
Copy Markdown
Member Author

I have to close that PR and open a new one to make it from the "apache/airflow" repository - there is a change in "breeze" that will only work when merged to main or when the PR is run from "apache/airflow" repo, so in order to test it, I need to recreate it.

@potiuk potiuk closed this Apr 8, 2023
@potiuk

potiuk commented Apr 8, 2023

Copy link
Copy Markdown
Member Author

New PR opened in #30544

@potiuk
potiuk deleted the further-fix-to-googleads-vendoring branch June 4, 2023 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools area:providers provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants