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 2023/01/09 23:06:09 UTC
[GitHub] [airflow] Taragolis opened a new pull request, #28820: Fix ArangoDB static checks
Taragolis opened a new pull request, #28820:
URL: https://github.com/apache/airflow/pull/28820
[python-arango == 7.5.5](https://github.com/ArangoDB-Community/python-arango/releases/tag/7.5.5) make some changes in types as result our static checks failed in main.
This PR fix static checks and type annotation for methods and properties
--
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 pull request #28820: Fix ArangoDB static checks
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28820:
URL: https://github.com/apache/airflow/pull/28820#issuecomment-1376836297
Nice. I was about to look at this one :)
--
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 merged pull request #28820: Fix ArangoDB static checks
Posted by GitBox <gi...@apache.org>.
eladkal merged PR #28820:
URL: https://github.com/apache/airflow/pull/28820
--
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] Taragolis commented on a diff in pull request #28820: Fix ArangoDB static checks
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28820:
URL: https://github.com/apache/airflow/pull/28820#discussion_r1065434735
##########
airflow/providers/arangodb/hooks/arangodb.py:
##########
@@ -18,14 +18,20 @@
"""This module allows connecting to a ArangoDB."""
from __future__ import annotations
-from typing import Any
+from typing import TYPE_CHECKING, Any
from arango import AQLQueryExecuteError, ArangoClient as ArangoDBClient
-from arango.result import Result
from airflow import AirflowException
+from airflow.compat.functools import cached_property
from airflow.hooks.base import BaseHook
+if TYPE_CHECKING:
Review Comment:
That for help mypy because arangodb methods return generic type Result[{Something}] which have definition
```python
T = TypeVar("T")
Result = Union[T, AsyncJob[T], BatchJob[T], None]
```
--
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 a diff in pull request #28820: Fix ArangoDB static checks
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28820:
URL: https://github.com/apache/airflow/pull/28820#discussion_r1065415994
##########
airflow/providers/arangodb/hooks/arangodb.py:
##########
@@ -18,14 +18,20 @@
"""This module allows connecting to a ArangoDB."""
from __future__ import annotations
-from typing import Any
+from typing import TYPE_CHECKING, Any
from arango import AQLQueryExecuteError, ArangoClient as ArangoDBClient
-from arango.result import Result
from airflow import AirflowException
+from airflow.compat.functools import cached_property
from airflow.hooks.base import BaseHook
+if TYPE_CHECKING:
Review Comment:
BTW. Why do we want to add "if TYPE_CHECKING" here @Taragolis ?
I was under the impression that we use if TYPE_CHECKING only when we cannot do it otherwise because of importing circular references but I think in this case it is not needed ?
--
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] Taragolis commented on a diff in pull request #28820: Fix ArangoDB static checks
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28820:
URL: https://github.com/apache/airflow/pull/28820#discussion_r1065455647
##########
airflow/providers/arangodb/hooks/arangodb.py:
##########
@@ -18,14 +18,20 @@
"""This module allows connecting to a ArangoDB."""
from __future__ import annotations
-from typing import Any
+from typing import TYPE_CHECKING, Any
from arango import AQLQueryExecuteError, ArangoClient as ArangoDBClient
-from arango.result import Result
from airflow import AirflowException
+from airflow.compat.functools import cached_property
from airflow.hooks.base import BaseHook
+if TYPE_CHECKING:
Review Comment:
And actual type known only in runtime because methods wraps it into additional handlers.
In this case it returns Cursor or raise some error, e.g.: we get [AQL.execute](https://github.com/ArangoDB-Community/python-arango/blob/1ab63c25c3b3fc152b8386c1b62c705645fb197b/arango/aql.py#L421-L424) which returns in our case [DefaultApiExecutor.execute](https://github.com/ArangoDB-Community/python-arango/blob/1ab63c25c3b3fc152b8386c1b62c705645fb197b/arango/executor.py#L55-L65)
--
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