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 2020/07/09 03:32:05 UTC
[GitHub] [airflow] zikun opened a new pull request #9730: DbApiHook: Support kwargs in get_pandas_df
zikun opened a new pull request #9730:
URL: https://github.com/apache/airflow/pull/9730
Support all parameters that are supported by pandas `read_sql` function: https://github.com/pandas-dev/pandas/blob/1.0.x/pandas/io/sql.py#L336-L345
Closes #8468
---
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Target Github ISSUE in description if exists
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] zikun commented on pull request #9730: DbApiHook: Support kwargs in get_pandas_df
Posted by GitBox <gi...@apache.org>.
zikun commented on pull request #9730:
URL: https://github.com/apache/airflow/pull/9730#issuecomment-670821557
@turbaszek let's merge it :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #9730: DbApiHook: Support kwargs in get_pandas_df
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9730:
URL: https://github.com/apache/airflow/pull/9730#discussion_r466209472
##########
File path: airflow/hooks/dbapi_hook.py
##########
@@ -106,7 +106,7 @@ def get_sqlalchemy_engine(self, engine_kwargs=None):
engine_kwargs = {}
return create_engine(self.get_uri(), **engine_kwargs)
- def get_pandas_df(self, sql, parameters=None):
+ def get_pandas_df(self, sql, parameters=None, **kwargs):
Review comment:
Would you mind adding information to method docstring?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #9730: DbApiHook: Support kwargs in get_pandas_df
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9730:
URL: https://github.com/apache/airflow/pull/9730#issuecomment-667647713
Looks good @zikun -> but there is a handful of other hooks that are defining this method and (hive/exasol) - and by a quick look they could also benefit from those kwargs. Would you mind adding them there as well ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] zikun edited a comment on pull request #9730: DbApiHook: Support kwargs in get_pandas_df
Posted by GitBox <gi...@apache.org>.
zikun edited a comment on pull request #9730:
URL: https://github.com/apache/airflow/pull/9730#issuecomment-655886583
Static check failed:
airflow/providers/google/cloud/hooks/bigquery.py:166: error: Signature of "get_pandas_df" incompatible with supertype "DbApiHook"
Added kwargs in BigQueryHook as well
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek merged pull request #9730: DbApiHook: Support kwargs in get_pandas_df
Posted by GitBox <gi...@apache.org>.
turbaszek merged pull request #9730:
URL: https://github.com/apache/airflow/pull/9730
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] zikun commented on pull request #9730: DbApiHook: Support kwargs in get_pandas_df
Posted by GitBox <gi...@apache.org>.
zikun commented on pull request #9730:
URL: https://github.com/apache/airflow/pull/9730#issuecomment-669625368
@potiuk Added to all the overriding methods
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] zikun commented on a change in pull request #9730: DbApiHook: Support kwargs in get_pandas_df
Posted by GitBox <gi...@apache.org>.
zikun commented on a change in pull request #9730:
URL: https://github.com/apache/airflow/pull/9730#discussion_r466395670
##########
File path: airflow/hooks/dbapi_hook.py
##########
@@ -106,7 +106,7 @@ def get_sqlalchemy_engine(self, engine_kwargs=None):
engine_kwargs = {}
return create_engine(self.get_uri(), **engine_kwargs)
- def get_pandas_df(self, sql, parameters=None):
+ def get_pandas_df(self, sql, parameters=None, **kwargs):
Review comment:
Added
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] zikun commented on pull request #9730: DbApiHook: Support kwargs in get_pandas_df
Posted by GitBox <gi...@apache.org>.
zikun commented on pull request #9730:
URL: https://github.com/apache/airflow/pull/9730#issuecomment-655886583
Static check failed:
airflow/providers/google/cloud/hooks/bigquery.py:166: error: Signature of "get_pandas_df" incompatible with supertype "DbApiHook"
Let me add kwargs in BigQueryHook as well
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org