uranusjr commented on a change in pull request #18692:
URL: https://github.com/apache/airflow/pull/18692#discussion_r721084950
##########
File path: airflow/models/connection.py
##########
@@ -408,3 +408,15 @@ def get_connection_from_secrets(cls, conn_id: str) ->
'Connection':
)
raise AirflowNotFoundException(f"The conn_id `{conn_id}` isn't
defined")
+
+ def __eq__(self, other: 'Connection'):
Review comment:
It's not recommended to annotate `__eq__` with a concrete type. The
recommended approach is something like this:
```python
def __eq__(self, other: object) -> bool: # I think Any would work as well.
if not isinstance(other, Connection):
return NotImplemented
# Actual comparison logic...
```
See also:
https://docs.python.org/3/library/numbers.html#implementing-the-arithmetic-operations
https://stackoverflow.com/questions/37557411/why-does-defining-the-argument-types-for-eq-throw-a-mypy-type-error
##########
File path: airflow/providers/amazon/aws/secrets/systems_manager.py
##########
@@ -16,15 +16,13 @@
# specific language governing permissions and limitations
# under the License.
"""Objects relating to sourcing connections from AWS SSM Parameter Store"""
+import json
from typing import Optional
import boto3
-try:
- from functools import cached_property
-except ImportError:
- from cached_property import cached_property
-
+from airflow.compat.functools import cached_property
Review comment:
The current try-except approach is intentional to keep the provider
compatible with older Airflow versions. It shouldn't be changed unless you're
going to bump the minimum Airflow requirement on this provider.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
