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/07/25 00:07:44 UTC
[GitHub] [airflow] edithturn opened a new pull request, #25268: D400 first line should end with period batch02
edithturn opened a new pull request, #25268:
URL: https://github.com/apache/airflow/pull/25268
Related to the issue: #10742
The second batch to fix the D400 | the First line should end with a period (That is being ignored in pre-commits).
I was having an issue with helm, after running `pre-commit run --all-files`, I didn't fix it, I pushed this code without that fix. It is related to this issue I described in Slack. https://apache-airflow.slack.com/archives/CCZRF2U5A/p1658682791405059.
All basic pre-commits run in the commit, so it lets me push. Appreciate any feedback!
--
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] edithturn commented on pull request #25268: D400 first line should end with period batch02
Posted by GitBox <gi...@apache.org>.
edithturn commented on PR #25268:
URL: https://github.com/apache/airflow/pull/25268#issuecomment-1228718642
@eladkal yep, I will fix this and send another batch this weekend :)
--
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 a diff in pull request #25268: D400 first line should end with period batch02
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25268:
URL: https://github.com/apache/airflow/pull/25268#discussion_r928476262
##########
airflow/providers/mysql/hooks/mysql.py:
##########
@@ -61,6 +61,8 @@ def __init__(self, *args, **kwargs) -> None:
def set_autocommit(self, conn: MySQLConnectionTypes, autocommit: bool) -> None:
"""
+ Mysqldb client uses an `autocommit` method rather than an `autocommit` property.
+
The MySQLdb (mysqlclient) client uses an `autocommit` method rather
Review Comment:
MySQLdb is the old name (Python 2 branch) of mysqlclient, the Python library to connect to MySQL. Since we don’t support Python 2 anymore, this should only mention mysqlclient.
##########
airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -149,6 +149,8 @@ async def __aexit__(self, *err):
@staticmethod
def _parse_host(host: str) -> str:
"""
+ Robustness to improper connections settings provided by users.
Review Comment:
I have trouble understanding what this means.
##########
airflow/providers/google/cloud/hooks/cloud_memorystore.py:
##########
@@ -392,8 +394,7 @@ def list_instances(
metadata: Sequence[Tuple[str, str]] = (),
):
"""
- Lists all Redis instances owned by a project in either the specified location (region) or all
- locations.
+ Lists all Redis instances owned by a project at the specified location (region) or all locations.
Review Comment:
```suggestion
Lists all Redis instances owned by a project at the specified location (region) or all locations.
```
##########
airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -175,21 +177,22 @@ def _parse_host(host: str) -> str:
def _get_retry_object(self) -> Retrying:
"""
- Instantiates a retry object
+ Instantiates a retry object.
:return: instance of Retrying class
"""
return Retrying(**self.retry_args)
def _a_get_retry_object(self) -> AsyncRetrying:
"""
- Instantiates an async retry object
+ Instantiates an async retry object.
:return: instance of AsyncRetrying class
"""
return AsyncRetrying(**self.retry_args)
def _get_aad_token(self, resource: str) -> str:
"""
- Function to get AAD token for given resource. Supports managed identity or service principal auth
+ Function to get AAD token for given resource.
+ Supports managed identity or service principal auth
Review Comment:
```suggestion
Function to get AAD token for given resource.
Supports managed identity or service principal auth.
```
I think there’s also a rule that the first line must not go over a certain length, and since reStructuredText has a line-collapsing rule, we need an empty line to prevent these being merged into a line.
##########
airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -597,7 +601,9 @@ def _retryable_error(exception: BaseException) -> bool:
class _TokenAuth(AuthBase):
"""
- Helper class for requests Auth field. AuthBase requires you to implement the __call__
+ Helper class for requests Auth field.
+
+ AuthBase requires you to implement the __call__
Review Comment:
```suggestion
AuthBase requires you to implement the ``__call__``
```
##########
airflow/providers/mysql/hooks/mysql.py:
##########
@@ -146,6 +150,8 @@ def _get_conn_config_mysql_connector_python(self, conn: Connection) -> Dict:
def get_conn(self) -> MySQLConnectionTypes:
"""
+ Connection to a mysql database.
Review Comment:
```suggestion
Connection to a MySQL database.
```
##########
airflow/utils/process_utils.py:
##########
@@ -190,6 +192,8 @@ def execute_in_subprocess_with_kwargs(cmd: List[str], **kwargs) -> None:
def execute_interactive(cmd: List[str], **kwargs) -> None:
"""
+ Runs the new command as a subproce.
Review Comment:
```suggestion
Runs the new command as a subprocess.
```
--
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 a diff in pull request #25268: D400 first line should end with period batch02
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25268:
URL: https://github.com/apache/airflow/pull/25268#discussion_r956916011
##########
airflow/providers/mysql/hooks/mysql.py:
##########
@@ -187,7 +193,7 @@ def bulk_load(self, table: str, tmp_file: str) -> None:
conn.close()
def bulk_dump(self, table: str, tmp_file: str) -> None:
- """Dumps a database table into a tab-delimited file"""
+ """Dumps a database table into a tab-delimited file."""
Review Comment:
```suggestion
"""Dump a database table into a tab-delimited file."""
```
##########
airflow/providers/mysql/hooks/mysql.py:
##########
@@ -174,7 +180,7 @@ def get_conn(self) -> MySQLConnectionTypes:
raise ValueError('Unknown MySQL client name provided!')
def bulk_load(self, table: str, tmp_file: str) -> None:
- """Loads a tab-delimited file into a database table"""
+ """Loads a tab-delimited file into a database table."""
Review Comment:
```suggestion
"""Load a tab-delimited file into a database table."""
```
--
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] edithturn commented on pull request #25268: D400 first line should end with period batch02
Posted by GitBox <gi...@apache.org>.
edithturn commented on PR #25268:
URL: https://github.com/apache/airflow/pull/25268#issuecomment-1231910314
@eladkal no, there are 3000 records to correct. I am sending it in parts since sending a single PR would not be adequate.
--
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] edithturn commented on pull request #25268: D400 first line should end with period batch02
Posted by GitBox <gi...@apache.org>.
edithturn commented on PR #25268:
URL: https://github.com/apache/airflow/pull/25268#issuecomment-1229727315
@uranusjr @eladkal sorry for the delay. Thank you for your feedback!
--
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 a diff in pull request #25268: D400 first line should end with period batch02
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25268:
URL: https://github.com/apache/airflow/pull/25268#discussion_r956896391
##########
airflow/providers/google/cloud/hooks/cloud_memorystore.py:
##########
@@ -752,8 +753,7 @@ def list_instances(
metadata: Sequence[Tuple[str, str]] = (),
):
"""
- Lists all Memcached instances owned by a project in either the specified location (region) or all
- locations.
+ Lists all Memcached instances owned by a projeca specified location (region) or all locations.
Review Comment:
```suggestion
List Memcached instances owned by a project at the specified location (region) or all locations.
```
##########
airflow/providers/mysql/hooks/mysql.py:
##########
@@ -61,7 +61,9 @@ def __init__(self, *args, **kwargs) -> None:
def set_autocommit(self, conn: MySQLConnectionTypes, autocommit: bool) -> None:
"""
- The MySQLdb (mysqlclient) client uses an `autocommit` method rather
+ Mysqlclient uses an `autocommit` method rather than an `autocommit` property.
Review Comment:
```suggestion
Set *autocommit*.
```
##########
airflow/providers/google/cloud/hooks/cloud_memorystore.py:
##########
@@ -392,8 +394,7 @@ def list_instances(
metadata: Sequence[Tuple[str, str]] = (),
):
"""
- Lists all Redis instances owned by a project in either the specified location (region) or all
- locations.
+ Lists all Redis instances owned by a project at the specified location (region) or all locations.
Review Comment:
```suggestion
List Redis instances owned by a project at the specified location (region) or all locations.
```
##########
airflow/providers/mysql/hooks/mysql.py:
##########
@@ -75,7 +77,9 @@ def set_autocommit(self, conn: MySQLConnectionTypes, autocommit: bool) -> None:
def get_autocommit(self, conn: MySQLConnectionTypes) -> bool:
"""
- The MySQLdb (mysqlclient) client uses a `get_autocommit` method
+ Mysqlclient uses a `get_autocommit` method rather than an `autocommit` property.
+
+ Mysqlclient client uses a `get_autocommit` method
rather than an `autocommit` property to get the autocommit setting
Review Comment:
```suggestion
*mysqlclient* uses an *get_autocommit* method rather than an *autocommit*
property, so we need to override this to support it.
```
##########
airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -195,21 +194,23 @@ def _parse_host(host: str) -> str:
def _get_retry_object(self) -> Retrying:
"""
- Instantiates a retry object
+ Instantiates a retry object.
Review Comment:
```suggestion
Instantiate a retry object.
```
##########
airflow/utils/process_utils.py:
##########
@@ -190,6 +192,8 @@ def execute_in_subprocess_with_kwargs(cmd: List[str], **kwargs) -> None:
def execute_interactive(cmd: List[str], **kwargs) -> None:
"""
+ Runs the new command as a subprocess.
Review Comment:
```suggestion
Run the new command as a subprocess.
```
##########
airflow/providers/mysql/hooks/mysql.py:
##########
@@ -202,6 +208,8 @@ def bulk_dump(self, table: str, tmp_file: str) -> None:
@staticmethod
def _serialize_cell(cell: object, conn: Optional[Connection] = None) -> object:
"""
+ Mysqldb converts argument to a literal.
Review Comment:
```suggestion
Convert argument to a literal.
```
##########
airflow/providers/google/cloud/hooks/cloud_memorystore.py:
##########
@@ -851,9 +851,10 @@ def update_parameters(
metadata: Sequence[Tuple[str, str]] = (),
):
"""
- Updates the defined Memcached Parameters for an existing Instance. This method only stages the
- parameters, it must be followed by apply_parameters to apply the parameters to nodes of
- the Memcached Instance.
+ Updates the defined Memcached Parameters for an existing Instance.
Review Comment:
```suggestion
Update the defined Memcached Parameters for an existing Instance.
```
##########
airflow/providers/mysql/hooks/mysql.py:
##########
@@ -75,7 +77,9 @@ def set_autocommit(self, conn: MySQLConnectionTypes, autocommit: bool) -> None:
def get_autocommit(self, conn: MySQLConnectionTypes) -> bool:
"""
- The MySQLdb (mysqlclient) client uses a `get_autocommit` method
+ Mysqlclient uses a `get_autocommit` method rather than an `autocommit` property.
Review Comment:
```suggestion
Whether *autocommit* is active.
```
##########
airflow/providers/mysql/hooks/mysql.py:
##########
@@ -61,7 +61,9 @@ def __init__(self, *args, **kwargs) -> None:
def set_autocommit(self, conn: MySQLConnectionTypes, autocommit: bool) -> None:
"""
- The MySQLdb (mysqlclient) client uses an `autocommit` method rather
+ Mysqlclient uses an `autocommit` method rather than an `autocommit` property.
+
+ Mysqlclient client uses an `autocommit` method rather
than an `autocommit` property to set the autocommit setting
Review Comment:
```suggestion
*mysqlclient* uses an *autocommit* method rather than an *autocommit*
property, so we need to override this to support it.
```
##########
airflow/providers/mysql/hooks/mysql.py:
##########
@@ -214,6 +222,8 @@ def _serialize_cell(cell: object, conn: Optional[Connection] = None) -> object:
def get_iam_token(self, conn: Connection) -> Tuple[str, int]:
"""
+ Awshook to retrieve a temporary password to connect to MySQL Port.
Review Comment:
```suggestion
Retrieve a temporary password to connect to MySQL.
```
##########
airflow/utils/process_utils.py:
##########
@@ -271,8 +275,9 @@ def kill_child_processes_by_pids(pids_to_kill: List[int], timeout: int = 5) -> N
@contextmanager
def patch_environ(new_env_variables: Dict[str, str]) -> Generator[None, None, None]:
"""
- Sets environment variables in context. After leaving the context, it restores its original state.
+ Sets environment variables in context.
Review Comment:
```suggestion
Set environment variables in context.
```
##########
airflow/providers/qubole/sensors/qubole.py:
##########
@@ -68,8 +68,9 @@ def poke(self, context: 'Context') -> bool:
class QuboleFileSensor(QuboleSensor):
"""
- Wait for a file or folder to be present in cloud storage
- and check for its presence via QDS APIs
+ Wait for a file or folder to be present in cloud storage.
+
+ Check for file or folder presence via QDS APIs
Review Comment:
```suggestion
Check for file or folder presence via QDS APIs.
```
##########
airflow/utils/process_utils.py:
##########
@@ -316,7 +321,7 @@ def check_if_pidfile_process_is_running(pid_file: str, process_name: str):
def set_new_process_group() -> None:
"""
- Tries to set current process to a new process group
+ Tries to set current process to a new process group.
Review Comment:
```suggestion
Try to set current process to a new process group.
```
##########
airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -195,21 +194,23 @@ def _parse_host(host: str) -> str:
def _get_retry_object(self) -> Retrying:
"""
- Instantiates a retry object
+ Instantiates a retry object.
:return: instance of Retrying class
"""
return Retrying(**self.retry_args)
def _a_get_retry_object(self) -> AsyncRetrying:
"""
- Instantiates an async retry object
+ Instantiates an async retry object.
Review Comment:
```suggestion
Instantiate an async retry object.
```
##########
airflow/providers/amazon/aws/transfers/redshift_to_s3.py:
##########
@@ -29,7 +29,7 @@
class RedshiftToS3Operator(BaseOperator):
"""
- Executes an UNLOAD command to s3 as a CSV with headers
+ Executes an UNLOAD command to s3 as a CSV with headers.
Review Comment:
```suggestion
Execute an UNLOAD command to s3 as a CSV with headers.
```
##########
airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -340,7 +341,7 @@ async def _a_get_aad_token(self, resource: str) -> str:
def _get_aad_headers(self) -> dict:
"""
- Fills AAD headers if necessary (SPN is outside of the workspace)
+ Fills AAD headers if necessary (SPN is outside of the workspace).
Review Comment:
```suggestion
Fill AAD headers if necessary (SPN is outside of the workspace).
```
--
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 a diff in pull request #25268: D400 first line should end with period batch02
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25268:
URL: https://github.com/apache/airflow/pull/25268#discussion_r928474323
##########
airflow/executors/executor_loader.py:
##########
@@ -134,7 +134,7 @@ def import_executor_cls(cls, executor_name: str) -> Tuple[Type["BaseExecutor"],
@classmethod
def __load_celery_kubernetes_executor(cls) -> "BaseExecutor":
- """:return: an instance of CeleryKubernetesExecutor"""
+ """:return: an instance of CeleryKubernetesExecutor."""
Review Comment:
These _return an instance of $Executor_ docstrings should simply be removed, they do not really communicate anything.
--
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 pull request #25268: D400 first line should end with period batch02
Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #25268:
URL: https://github.com/apache/airflow/pull/25268#issuecomment-1228715026
@edithturn are you still working on this PR?
--
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 pull request #25268: D400 first line should end with period batch02
Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #25268:
URL: https://github.com/apache/airflow/pull/25268#issuecomment-1231907541
Are these all the fixes?
Can we remove D400 from
https://github.com/apache/airflow/blob/733d62317c62120506edff1bd66d3343b4df88c8/.pre-commit-config.yaml#L258
to activate the this doc style?
--
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 #25268: D400 first line should end with period batch02
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #25268:
URL: https://github.com/apache/airflow/pull/25268
--
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