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 2021/02/22 17:21:32 UTC
[GitHub] [airflow] wpromatt opened a new issue #14363: Argument order change in airflow.providers.google.cloud.hooks.gcs.GCSHook:download appears to be a mistake.
wpromatt opened a new issue #14363:
URL: https://github.com/apache/airflow/issues/14363
https://github.com/apache/airflow/blob/6019c78cb475800f58714a9dabb747b9415599c8/airflow/providers/google/cloud/hooks/gcs.py#L262-L265
Was this order swap of the `object_name` and `bucket_name` arguments in the `GCSHook.download` a mistake? The `upload` and `delete` methods still use `bucket_name` first and the commit where this change was made `1845cd11b77f302777ab854e84bef9c212c604a0` was supposed to just add strict type checking. The docstring also appears to reference the old order.
----------------------------------------------------------------
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] mlgruby edited a comment on issue #14363: Argument order change in airflow.providers.google.cloud.hooks.gcs.GCSHook:download appears to be a mistake.
Posted by GitBox <gi...@apache.org>.
mlgruby edited a comment on issue #14363:
URL: https://github.com/apache/airflow/issues/14363#issuecomment-783816698
Yes, the swap was done on purpose as there is no default value for object_name and to make mypy pass this check I have to put the no default argument at first and then the default value arguments ones.
Agree that the doc block is not updated, My Bad. I will make changes to the doc block and also make changes to upload and delete so that all three functions can have the same order of arguments.
----------------------------------------------------------------
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] mlgruby commented on issue #14363: Argument order change in airflow.providers.google.cloud.hooks.gcs.GCSHook:download appears to be a mistake.
Posted by GitBox <gi...@apache.org>.
mlgruby commented on issue #14363:
URL: https://github.com/apache/airflow/issues/14363#issuecomment-783816698
Yes, the swap was done on purpose as there is no default value for object_name and to make mypy pass this check I have to put the no default argument at first and then the default value arguments ones.
Agree that the doc block is not updated, My Bad.
----------------------------------------------------------------
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] mlgruby edited a comment on issue #14363: Argument order change in airflow.providers.google.cloud.hooks.gcs.GCSHook:download appears to be a mistake.
Posted by GitBox <gi...@apache.org>.
mlgruby edited a comment on issue #14363:
URL: https://github.com/apache/airflow/issues/14363#issuecomment-783816698
Yes, the swap was done on purpose as there is no default value for object_name and to make mypy pass this check I have to put the no default argument at first and then the default value arguments ones.
Agree that the doc block is not updated, My Bad. I make the changes to doc block and also make changes to upload and delete so that all three functions can have the same order of arguments.
----------------------------------------------------------------
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 #14363: Argument order change in airflow.providers.google.cloud.hooks.gcs.GCSHook:download appears to be a mistake.
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #14363:
URL: https://github.com/apache/airflow/issues/14363#issuecomment-783534747
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 #14363: Argument order change in airflow.providers.google.cloud.hooks.gcs.GCSHook:download appears to be a mistake.
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on issue #14363:
URL: https://github.com/apache/airflow/issues/14363#issuecomment-783843935
Thanks for the info @mlgruby! Makes sense.
@wpromatt if you'd like to get a PR with Airflow you can update the doc block, otherwise I'm happy to make the change.
----------------------------------------------------------------
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 #14363: Argument order change in airflow.providers.google.cloud.hooks.gcs.GCSHook:download appears to be a mistake.
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on issue #14363:
URL: https://github.com/apache/airflow/issues/14363#issuecomment-783794576
I suspect this was done on purpose, since the change also made `bucket_name` optional, and `object_name` is still required. But you're right about the doc-block, that should be updated to match the current order.
@mlgruby does this sound correct?
----------------------------------------------------------------
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 closed issue #14363: Argument order change in airflow.providers.google.cloud.hooks.gcs.GCSHook:download appears to be a mistake.
Posted by GitBox <gi...@apache.org>.
turbaszek closed issue #14363:
URL: https://github.com/apache/airflow/issues/14363
----------------------------------------------------------------
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