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/01/04 22:04:50 UTC
[GitHub] [airflow] ferruzzi opened a new pull request #20369: Standardize AWS Batch naming
ferruzzi opened a new pull request #20369:
URL: https://github.com/apache/airflow/pull/20369
Part of https://github.com/apache/airflow/issues/20296
In addition:
- Fixed capitalization in some comments from `batch` to `Batch` where applicable.
- Changed `execute(None)` and `poke(None)` to `execute({})` and `poke({})` because my IDE was throwing a type error (expected a dict, got NoneType).
<!--
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 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 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/main/UPDATING.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 merged pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #20369:
URL: https://github.com/apache/airflow/pull/20369
--
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] eladkal commented on a change in pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20369:
URL: https://github.com/apache/airflow/pull/20369#discussion_r778378157
##########
File path: airflow/providers/amazon/aws/hooks/batch_client.py
##########
@@ -538,3 +538,35 @@ def exp(tries):
delay = 1 + pow(tries * 0.6, 2)
delay = min(max_interval, delay)
return uniform(delay / 3, delay)
+
+
+class AwsBatchProtocol(BatchProtocol, Protocol):
+ """
+ This class is deprecated.
+ Please use :class:`airflow.providers.amazon.aws.hooks.batch.BatchProtocol`.
+ """
+
+ def __init__(self, *args, **kwargs):
+ warnings.warn(
+ "This class is deprecated. "
+ "Please use :class:`airflow.providers.amazon.aws.hooks.batch.BatchProtocol`.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+ super().__init__(*args, **kwargs)
Review comment:
probably not needed any more...
--
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] ferruzzi commented on pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on pull request #20369:
URL: https://github.com/apache/airflow/pull/20369#issuecomment-1006225381
Now we are failing isort because of https://github.com/apache/airflow/pull/20369#discussion_r778602745 and removing the Protocol inheritance in https://github.com/apache/airflow/pull/20369/commits/05abb3bd428be76278ac74bcf78a783853905e5d [causes the error](https://github.com/apache/airflow/runs/4722050328?check_suite_focus=true#step:6:6209) we saw before about "only a Protocol can import a Protocol"
I'm not sure what to do here.
--
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] ferruzzi closed pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
ferruzzi closed pull request #20369:
URL: https://github.com/apache/airflow/pull/20369
--
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] eladkal commented on a change in pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20369:
URL: https://github.com/apache/airflow/pull/20369#discussion_r779115559
##########
File path: airflow/providers/amazon/aws/hooks/batch_client.py
##########
@@ -538,3 +538,35 @@ def exp(tries):
delay = 1 + pow(tries * 0.6, 2)
delay = min(max_interval, delay)
return uniform(delay / 3, delay)
+
+
+class AwsBatchProtocol(BatchProtocol, Protocol):
+ """
+ This class is deprecated.
+ Please use :class:`airflow.providers.amazon.aws.hooks.batch.BatchProtocol`.
+ """
+
+ def __init__(self, *args, **kwargs):
+ warnings.warn(
+ "This class is deprecated. "
+ "Please use :class:`airflow.providers.amazon.aws.hooks.batch.BatchProtocol`.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+ super().__init__(*args, **kwargs)
Review comment:
You were right. We need the protocol.
see a fix we had to deploy https://github.com/apache/airflow/pull/20669
--
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] ferruzzi commented on a change in pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on a change in pull request #20369:
URL: https://github.com/apache/airflow/pull/20369#discussion_r771158731
##########
File path: airflow/providers/amazon/aws/hooks/batch_client.py
##########
@@ -538,3 +538,35 @@ def exp(tries):
delay = 1 + pow(tries * 0.6, 2)
delay = min(max_interval, delay)
return uniform(delay / 3, delay)
+
+
+class AwsBatchProtocol(BatchProtocol, Protocol):
+ """
+ This class is deprecated.
+ Please use :class:`airflow.providers.amazon.aws.hooks.batch.BatchProtocol`.
+ """
+
+ def __init__(self, *args, **kwargs):
+ warnings.warn(
+ "This class is deprecated. "
+ "Please use :class:`airflow.providers.amazon.aws.hooks.batch.BatchProtocol`.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+ super().__init__(*args, **kwargs)
Review comment:
Please see this discussion before approving: https://github.com/apache/airflow/pull/20332#discussion_r770179588
--
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] ferruzzi commented on a change in pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on a change in pull request #20369:
URL: https://github.com/apache/airflow/pull/20369#discussion_r778387027
##########
File path: airflow/providers/amazon/aws/hooks/batch_client.py
##########
@@ -538,3 +538,35 @@ def exp(tries):
delay = 1 + pow(tries * 0.6, 2)
delay = min(max_interval, delay)
return uniform(delay / 3, delay)
+
+
+class AwsBatchProtocol(BatchProtocol, Protocol):
+ """
+ This class is deprecated.
+ Please use :class:`airflow.providers.amazon.aws.hooks.batch.BatchProtocol`.
+ """
+
+ def __init__(self, *args, **kwargs):
+ warnings.warn(
+ "This class is deprecated. "
+ "Please use :class:`airflow.providers.amazon.aws.hooks.batch.BatchProtocol`.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+ super().__init__(*args, **kwargs)
Review comment:
That is correct, based on the other one now working I'll be doing this next. I'm not sure why that was giving me an error before, but it seems clear now. /shrug
--
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] ferruzzi closed pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
ferruzzi closed pull request #20369:
URL: https://github.com/apache/airflow/pull/20369
--
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] eladkal commented on a change in pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20369:
URL: https://github.com/apache/airflow/pull/20369#discussion_r778602745
##########
File path: airflow/providers/amazon/aws/operators/batch.py
##########
@@ -26,17 +26,18 @@
- http://boto3.readthedocs.io/en/latest/reference/services/batch.html
- https://docs.aws.amazon.com/batch/latest/APIReference/Welcome.html
"""
-from typing import TYPE_CHECKING, Any, Optional, Sequence
+import warnings
+from typing import Any, Optional, Sequence, TYPE_CHECKING
Review comment:
```suggestion
from typing import TYPE_CHECKING, Any, Optional, Sequence
```
to fix static checks
--
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] github-actions[bot] commented on pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20369:
URL: https://github.com/apache/airflow/pull/20369#issuecomment-1006924011
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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] ferruzzi commented on a change in pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on a change in pull request #20369:
URL: https://github.com/apache/airflow/pull/20369#discussion_r771157228
##########
File path: airflow/providers/amazon/aws/hooks/batch_client.py
##########
@@ -39,10 +39,10 @@
@runtime_checkable
-class AwsBatchProtocol(Protocol):
+class BatchProtocol(Protocol):
"""
A structured Protocol for ``boto3.client('batch') -> botocore.client.Batch``.
- This is used for type hints on :py:meth:`.AwsBatchClient.client`; it covers
+ This is used for type hints on :py:meth:`.BatchClient.client`; it covers
only the subset of client methods required.
Review comment:
I'm not 100% sure if this one should have been left along. Please double check me here.
--
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] eladkal commented on a change in pull request #20369: Standardize AWS Batch naming
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20369:
URL: https://github.com/apache/airflow/pull/20369#discussion_r779515713
##########
File path: airflow/providers/amazon/aws/operators/batch.py
##########
@@ -26,17 +26,18 @@
- http://boto3.readthedocs.io/en/latest/reference/services/batch.html
- https://docs.aws.amazon.com/batch/latest/APIReference/Welcome.html
"""
-from typing import TYPE_CHECKING, Any, Optional, Sequence
+import warnings
+from typing import Any, Optional, Sequence, TYPE_CHECKING
Review comment:
The previous code was correct. This change fails static checks
--
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