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/23 13:53:32 UTC
[GitHub] [airflow] JCoder01 opened a new issue #9957: dbapi_hook get_records doesn't accept list.
JCoder01 opened a new issue #9957:
URL: https://github.com/apache/airflow/issues/9957
**Apache Airflow version**:1.10.10
**Kubernetes version (if you are using kubernetes)** (use `kubectl version`):
**Environment**:
- **Cloud provider or hardware configuration**:
- **OS** (e.g. from /etc/os-release):
- **Kernel** (e.g. `uname -a`):
- **Install tools**:
- **Others**:
**What happened**:
passing a list to `dbapi_hook.get_records` fails
`TypeError: argument 1 must be a string or unicode object: got list instead`
**What you expected to happen**:
return list of query results.
**How to reproduce it**:
```
from airflow.hooks.postgres_hook import PostgresHook
PostgresHook().get_records(['select * from dag_run limit 1', 'select * from dag_run limit 1'])
'''
**Anything else we need to know**:
----------------------------------------------------------------
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] JCoder01 commented on issue #9957: dbapi_hook get_records doesn't accept list.
Posted by GitBox <gi...@apache.org>.
JCoder01 commented on issue #9957:
URL: https://github.com/apache/airflow/issues/9957#issuecomment-679959620
@jhtimmins is your question whether we should iterate over the sql statements and the parameters, or just iterate over the sql statements and pass in the same parameters to each sql item? My belief is that it should be the latter, and how it has been implemented in the PR.
----------------------------------------------------------------
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] JCoder01 edited a comment on issue #9957: dbapi_hook get_records doesn't accept list.
Posted by GitBox <gi...@apache.org>.
JCoder01 edited a comment on issue #9957:
URL: https://github.com/apache/airflow/issues/9957#issuecomment-679959620
@jhtimmins is your question whether we should iterate over the sql statements and the parameters, or just iterate over the sql statements and pass in the same parameters to each sql item? My belief is that it should be the latter, and how it has been implemented in the PR. https://github.com/apache/airflow/pull/9960
----------------------------------------------------------------
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] pkonarzewski commented on issue #9957: dbapi_hook get_records doesn't accept list.
Posted by GitBox <gi...@apache.org>.
pkonarzewski commented on issue #9957:
URL: https://github.com/apache/airflow/issues/9957#issuecomment-714601747
I'd like to try to fix 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] mik-laj commented on issue #9957: dbapi_hook get_records doesn't accept list.
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #9957:
URL: https://github.com/apache/airflow/issues/9957#issuecomment-714770290
@pkonarzewski I assigned you to this ticket 🐈
----------------------------------------------------------------
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] boring-cyborg[bot] commented on issue #9957: dbapi_hook get_records doesn't accept list.
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #9957:
URL: https://github.com/apache/airflow/issues/9957#issuecomment-663020129
Thanks for opening your first issue here! Be sure to follow the issue template!
----------------------------------------------------------------
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] jhtimmins commented on issue #9957: dbapi_hook get_records doesn't accept list.
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on issue #9957:
URL: https://github.com/apache/airflow/issues/9957#issuecomment-665340158
I was able to replicate this bug. Sqlalchemy expects to receive a string, not a list. It would be trivial to iterate over the input statements and run them in sequence.
However, it's not clear to me what the original plan for this method was, since it accepts a list or dictionary of parameters. SqlAlchemy has a pattern in which a parameterized query string is passed in, and then a list of dicts/lists containing parameters is as passed well. The query is run against all of the parameters individually. But if `get_records` accept both a list of queries _and_ a list of parameters, it's not clear if or how the parameters should be applied to individual query statements.
@kaxil what do you think?
----------------------------------------------------------------
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] pkonarzewski commented on issue #9957: dbapi_hook get_records doesn't accept list.
Posted by GitBox <gi...@apache.org>.
pkonarzewski commented on issue #9957:
URL: https://github.com/apache/airflow/issues/9957#issuecomment-715917177
So I've wrote some code to fix this issue, but now i have some doubts.
It's a simple fix, but there is few corner cases, that make it a little bit messy for me.
My problems are:
1. get_records() with one sql returns List[Tuple], with list of sqls it will return List[List[Tuple]], is this inconsistency ok?
2. get_first() returns Tuple, should multi sql statements return List[Tuple]? Same problem as in point 1.
3. what to do when one of the queries from list returns empty result?
4. there is also method get_pandas_df(), shouldn't it also allow multiple sql statements?
What do you think?
----------------------------------------------------------------
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] kaxil commented on issue #9957: dbapi_hook get_records doesn't accept list.
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #9957:
URL: https://github.com/apache/airflow/issues/9957#issuecomment-665614789
Based on https://www.psycopg.org/docs/cursor.html#cursor.execute and https://dev.mysql.com/doc/connector-python/en/connector-python-api-mysqlcursor-execute.html it looks like the cursor.execute method is up[posed to just take a string (query to execute).
----------------------------------------------------------------
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