Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Dec 15, 2021

Part of #19891


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

@potiuk potiuk requested a review from turbaszek as a code owner December 15, 2021 20:37
@boring-cyborg boring-cyborg bot added area:providers provider:google Google (including GCP) related issues labels Dec 15, 2021
@potiuk potiuk force-pushed the fix-mypy-google-bigquery-errors branch from 5830441 to 869ba74 Compare December 15, 2021 23:27
@potiuk potiuk requested a review from jedcunningham December 15, 2021 23:28
@potiuk potiuk added the mypy Fixing MyPy problems after bumpin MyPy to 0.990 label Dec 15, 2021
@potiuk potiuk force-pushed the fix-mypy-google-bigquery-errors branch from 869ba74 to c38d741 Compare December 16, 2021 00:56
@potiuk potiuk mentioned this pull request Dec 16, 2021
10 tasks
@potiuk potiuk force-pushed the fix-mypy-google-bigquery-errors branch from c38d741 to 1fa04d2 Compare December 16, 2021 12:49
@potiuk
Copy link
Member Author

potiuk commented Dec 16, 2021

One of the last ones for google (I think we have 50 left after this is merged :))

@potiuk
Copy link
Member Author

potiuk commented Dec 16, 2021

likewise looking for reviewer :)

@potiuk potiuk force-pushed the fix-mypy-google-bigquery-errors branch from 1fa04d2 to 89b27f7 Compare December 18, 2021 19:13
Copy link
Contributor

Choose a reason for hiding this comment

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

I recently changed gcs_hooks's return value as the solution to an issue.
So if you are not passing the filename to the download() method it will return only bytes otherwise file name.
Therefore the variable schema_fields_bytes_or_string always will be as bytes, but unfortunately gcs_hook.download's return type is bytes | str.

So how do you think could we skip this check if hasattr(schema_fields_bytes_or_string, 'decode')?

Also, maybe we could use here something like that?
gcs_hook.get_conn().bucket(gcs_bucket).blob(blob_name=gcs_object).download_as_bytes()

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I changed it in the way that I created a separate method in the hook that only returns bytes (and does not pass filename) and cast it to bytes - with appropriate comment.

@potiuk potiuk force-pushed the fix-mypy-google-bigquery-errors branch from 89b27f7 to a61dbee Compare December 21, 2021 14:37
@potiuk
Copy link
Member Author

potiuk commented Dec 21, 2021

Would love to merge all google stuff !

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Dec 21, 2021
@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@potiuk potiuk merged commit c6dbb3f into apache:main Dec 21, 2021
@potiuk potiuk deleted the fix-mypy-google-bigquery-errors branch December 21, 2021 16:31
@kaxil kaxil added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jan 13, 2022
@potiuk potiuk restored the fix-mypy-google-bigquery-errors branch April 26, 2022 20:48
@potiuk potiuk deleted the fix-mypy-google-bigquery-errors branch July 29, 2022 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge mypy Fixing MyPy problems after bumpin MyPy to 0.990 provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants