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/12/21 22:59:09 UTC
[GitHub] [airflow] kaxil opened a new issue #20457: Release 3.0.0 of Amazon Provider
kaxil opened a new issue #20457:
URL: https://github.com/apache/airflow/issues/20457
### Apache Airflow version
2.2.3 (latest released)
### What happened
The 2.5.0 version of Amazon Provider contains breaking changes - https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/commits.html specifically https://github.com/apache/airflow/commit/83b51e53062dc596a630edd4bd01407a556f1aa6 confirmed by @uranusjr
I have yanked 2.5.0 and we will need to release 3.0.0
### What you expected to happen
_No response_
### How to reproduce
_No response_
### Operating System
e
### Versions of Apache Airflow Providers
_No response_
### Deployment
Other 3rd-party Helm chart
### Deployment details
_No response_
### Anything else
_No response_
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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 commented on issue #20457: Release 3.0.0 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #20457:
URL: https://github.com/apache/airflow/issues/20457#issuecomment-999161037
What kind of breaking changes they are @uranusjr - they are not immediately visible. Can we make it baclwards-scompatible instead? I would not introduce backwards incompatibility unless we really have a good reason.
--
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 edited a comment on issue #20457: Release 3.0.0 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #20457:
URL: https://github.com/apache/airflow/issues/20457#issuecomment-999190957
> More deeply though, `expand_role()` used to depend on boto’s `get_client_type()` with a dynamic client type, which is deprecated in newer Boto versions. Tt now uses `_get_credentials()` instead to avoid the deprecation. But `expand_role()` does not tak a region argument and therefore cannot pass one to `_get_credentials()`. If we keep compatibility on `_get_credentials()`, we’d break compatibility on `expand_role()` instead.
Can't we pass `None` explicitly as `region` from the `expand_role()` and leave the `region` argument Optional but without default ?
--
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 commented on issue #20457: Release 3.0.0 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #20457:
URL: https://github.com/apache/airflow/issues/20457#issuecomment-999375663
Yeah. Seems to small af a thing to make whole provider backwards incompatible.
--
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 edited a comment on issue #20457: Release 3.0.0 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #20457:
URL: https://github.com/apache/airflow/issues/20457#issuecomment-999161037
What kind of breaking changes they are @uranusjr ?
They are not immediately visible. Can we make it backwards-compatible instead?
I would not introduce backwards incompatibility unless we really have a good reason.
--
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 issue #20457: Release 3.0.0 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #20457:
URL: https://github.com/apache/airflow/issues/20457#issuecomment-999185822
On the surface level, the `AwsBaseHook.expand_role()` now calls `_get_credentials()` with no arguments, which is not supported on older versions, and thus if someone subclasses the hook to reimplement `_get_credentials()`, the code would break after upgrade.
More deeply though, `expand_role()` used to depend on boto’s `get_client_type()` with a dynamic client type, which is deprecated in newer Boto versions. Tt now uses `_get_credentials()` instead to avoid the deprecation. But `expand_role()` does not tak a region argument and therefore cannot pass one to `_get_credentials()`. If we keep compatibility on `_get_credentials()`, we’d break compatibility on `expand_role()` instead.
--
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 closed issue #20457: Release 2.5.1 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #20457:
URL: https://github.com/apache/airflow/issues/20457
--
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 commented on issue #20457: Release 3.0.0 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #20457:
URL: https://github.com/apache/airflow/issues/20457#issuecomment-999190957
> More deeply though, `expand_role()` used to depend on boto’s `get_client_type()` with a dynamic client type, which is deprecated in newer Boto versions. Tt now uses `_get_credentials()` instead to avoid the deprecation. But `expand_role()` does not tak a region argument and therefore cannot pass one to `_get_credentials()`. If we keep compatibility on `_get_credentials()`, we’d break compatibility on `expand_role()` instead.
Can't we pass `None` explicitly as region from the `expand_role()` and leave the argument Optional but without default ?
--
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 edited a comment on issue #20457: Release 3.0.0 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #20457:
URL: https://github.com/apache/airflow/issues/20457#issuecomment-999161037
What kind of breaking changes they are @uranusjr - they are not immediately visible. Can we make it baclwards-compatible instead? I would not introduce backwards incompatibility unless we really have a good reason.
--
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 edited a comment on issue #20457: Release 3.0.0 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #20457:
URL: https://github.com/apache/airflow/issues/20457#issuecomment-999190957
> More deeply though, `expand_role()` used to depend on boto’s `get_client_type()` with a dynamic client type, which is deprecated in newer Boto versions. Tt now uses `_get_credentials()` instead to avoid the deprecation. But `expand_role()` does not tak a region argument and therefore cannot pass one to `_get_credentials()`. If we keep compatibility on `_get_credentials()`, we’d break compatibility on `expand_role()` instead.
Can't we pass `None` explicitly as region from the `expand_role()` and leave the `region` argument Optional but without default ?
--
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] kaxil commented on issue #20457: Release 3.0.0 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #20457:
URL: https://github.com/apache/airflow/issues/20457#issuecomment-999156040
cc @potiuk
--
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 issue #20457: Release 3.0.0 of Amazon Provider
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #20457:
URL: https://github.com/apache/airflow/issues/20457#issuecomment-999197192
Hmm, I think so. And maybe `expand_role()` should gain an optional `region_name` argument as well (defaulting to None and never explicitly used in the provider until 3.0).
--
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