You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/11/07 12:37:14 UTC
[GitHub] [airflow] pankajastro opened a new pull request, #27543: Rename hook bigquery function `_bq_cast` to `bq_cast`
pankajastro opened a new pull request, #27543:
URL: https://github.com/apache/airflow/pull/27543
Recently, I have started using hook bigquery function `_bq_cast` but I'm afraid that this might get changed silently since it is a private function so I want to rename it to `bq_cast` and deprecate `_bq_cast`.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
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](https://github.com/apache/airflow/tree/main/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pankajastro commented on pull request #27543: Rename hook bigquery function `_bq_cast` to `bq_cast`
Posted by GitBox <gi...@apache.org>.
pankajastro commented on PR #27543:
URL: https://github.com/apache/airflow/pull/27543#issuecomment-1306983090
> If we decide to make this function public, I also wonder if it’d be a good idea to move the function to somewhere in `utils` instead.
I was not able to find the right place to add it to utils. shall I create `utils/provider.py` or `utils/type_cast.py`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #27543: Rename hook bigquery function `_bq_cast` to `bq_cast`
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #27543:
URL: https://github.com/apache/airflow/pull/27543#issuecomment-1308271671
Maybe `airflow.providers.google.cloud.utils.bigquery`? There’s already a `bigquery_get_data` so _maybe_ it might make sense in there too…? Not sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pankajastro commented on a diff in pull request #27543: Rename hook bigquery function `_bq_cast` to `bq_cast`
Posted by GitBox <gi...@apache.org>.
pankajastro commented on code in PR #27543:
URL: https://github.com/apache/airflow/pull/27543#discussion_r1016432247
##########
airflow/providers/google/cloud/hooks/bigquery.py:
##########
@@ -2844,6 +2844,18 @@ def _escape(s: str) -> str:
def _bq_cast(string_field: str, bq_type: str) -> None | int | float | bool | str:
+ """
+ Helper method that casts a BigQuery row to the appropriate data types.
+ This is useful because BigQuery returns all fields as strings.
+ """
+ warnings.warn(
+ "This function is deprecated and will be Remove in future. Please use `bq_cast` function",
+ DeprecationWarning,
+ )
+ return bq_cast(string_field=string_field, bq_type=bq_type)
Review Comment:
hmm, just wanted to avoid breaking any external lib depending on it. I have removed this warning 5c7ef1579af5d3d4a05d83fa43488dd4de9ac8f6
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #27543: Rename hook bigquery function `_bq_cast` to `bq_cast`
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #27543:
URL: https://github.com/apache/airflow/pull/27543#discussion_r1015390612
##########
airflow/providers/google/cloud/hooks/bigquery.py:
##########
@@ -2844,6 +2844,18 @@ def _escape(s: str) -> str:
def _bq_cast(string_field: str, bq_type: str) -> None | int | float | bool | str:
+ """
+ Helper method that casts a BigQuery row to the appropriate data types.
+ This is useful because BigQuery returns all fields as strings.
+ """
+ warnings.warn(
+ "This function is deprecated and will be Remove in future. Please use `bq_cast` function",
+ DeprecationWarning,
+ )
+ return bq_cast(string_field=string_field, bq_type=bq_type)
Review Comment:
I’d not do this at all, the function is private and not covered by backward compatibility policies.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #27543: Rename hook bigquery function `_bq_cast` to `bq_cast`
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #27543:
URL: https://github.com/apache/airflow/pull/27543
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pankajastro commented on pull request #27543: Rename hook bigquery function `_bq_cast` to `bq_cast`
Posted by GitBox <gi...@apache.org>.
pankajastro commented on PR #27543:
URL: https://github.com/apache/airflow/pull/27543#issuecomment-1308289550
> Maybe `airflow.providers.google.cloud.utils.bigquery`? There’s already a `bigquery_get_data` so _maybe_ it might make sense in there too…? Not sure.
for this PR scope, I'll be in favour of keeping it in `airflow.providers.google.cloud.utils.bigquery` latter maybe we can also move `bigquery_get_data` in `airflow.providers.google.cloud.utils.bigquery`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org