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/04/10 15:10:06 UTC
[GitHub] [airflow] mik-laj opened a new pull request, #22887: Bump pre-commit hook versions
mik-laj opened a new pull request, #22887:
URL: https://github.com/apache/airflow/pull/22887
<!--
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 commented on a diff in pull request #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #22887:
URL: https://github.com/apache/airflow/pull/22887#discussion_r849830654
##########
airflow/providers/influxdb/hooks/influxdb.py:
##########
@@ -68,11 +68,9 @@ def get_uri(self, conn: Connection):
based on SSL or other InfluxDB host requirements
"""
- return '{scheme}://{host}:{port}'.format(
- scheme='https' if conn.schema is None else f'{conn.schema}',
- host=conn.host,
- port='7687' if conn.port is None else f'{conn.port}',
- )
+ conn_scheme = 'https' if conn.schema is None else f'{conn.schema}'
+ conn_port = '7687' if conn.port is None else f'{conn.port}'
+ return f"{conn_scheme}://{conn.host}:{conn_port}"
Review Comment:
Nice :)
--
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 #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #22887:
URL: https://github.com/apache/airflow/pull/22887#discussion_r846966566
##########
airflow/operators/subdag.py:
##########
@@ -115,14 +115,9 @@ def _validate_pool(self, session):
pool = session.query(Pool).filter(Pool.slots == 1).filter(Pool.pool == self.pool).first()
if pool and any(t.pool == self.pool for t in self.subdag.tasks):
raise AirflowException(
- 'SubDagOperator {sd} and subdag task{plural} {t} both '
- 'use pool {p}, but the pool only has 1 slot. The '
- 'subdag tasks will never run.'.format(
- sd=self.task_id,
- plural=len(conflicts) > 1,
- t=', '.join(t.task_id for t in conflicts),
- p=self.pool,
- )
+ f"SubDagOperator {self.task_id} and subdag task{len(conflicts) > 1} "
+ f"{', '.join(t.task_id for t in conflicts)} both use pool {self.pool}, "
+ f"but the pool only has 1 slot. The subdag tasks will never run."
Review Comment:
Wait this `task{len(conflicts) > 1}` does not look right. It seem it’s wrong since a long time ago but let’s fix it.
--
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 #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #22887:
URL: https://github.com/apache/airflow/pull/22887#discussion_r846967088
##########
airflow/providers/influxdb/hooks/influxdb.py:
##########
@@ -68,11 +68,9 @@ def get_uri(self, conn: Connection):
based on SSL or other InfluxDB host requirements
"""
- return '{scheme}://{host}:{port}'.format(
- scheme='https' if conn.schema is None else f'{conn.schema}',
- host=conn.host,
- port='7687' if conn.port is None else f'{conn.port}',
- )
+ conn_scheme = 'https' if conn.schema is None else f'{conn.schema}'
+ conn_port = '7687' if conn.port is None else f'{conn.port}'
+ return f"{conn_scheme}://{conn.host}:{conn_port}"
Review Comment:
```suggestion
conn_scheme = 'https' if conn.schema is None else conn.schema
conn_port = 7687 if conn.port is None else conn.port
return f"{conn_scheme}://{conn.host}:{conn_port}"
```
--
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 #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22887:
URL: https://github.com/apache/airflow/pull/22887#issuecomment-1094379384
needs fixes and rebase :)
--
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 #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #22887:
URL: https://github.com/apache/airflow/pull/22887#discussion_r846967088
##########
airflow/providers/influxdb/hooks/influxdb.py:
##########
@@ -68,11 +68,9 @@ def get_uri(self, conn: Connection):
based on SSL or other InfluxDB host requirements
"""
- return '{scheme}://{host}:{port}'.format(
- scheme='https' if conn.schema is None else f'{conn.schema}',
- host=conn.host,
- port='7687' if conn.port is None else f'{conn.port}',
- )
+ conn_scheme = 'https' if conn.schema is None else f'{conn.schema}'
+ conn_port = '7687' if conn.port is None else f'{conn.port}'
+ return f"{conn_scheme}://{conn.host}:{conn_port}"
Review Comment:
```suggestion
conn_scheme = 'https' if conn.schema is None else conn.schema
conn_port = '7687' if conn.port is None else conn.port
return f"{conn_scheme}://{conn.host}:{conn_port}"
```
--
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 #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22887:
URL: https://github.com/apache/airflow/pull/22887#issuecomment-1108897045
Can you rebase @mik-laj pls :) (last time ?)
--
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] mik-laj commented on pull request #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
mik-laj commented on PR #22887:
URL: https://github.com/apache/airflow/pull/22887#issuecomment-1100853712
@potiuk @uranusjr All green. 0 conflicts.
--
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] mik-laj merged pull request #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
mik-laj merged PR #22887:
URL: https://github.com/apache/airflow/pull/22887
--
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 #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #22887:
URL: https://github.com/apache/airflow/pull/22887#discussion_r846967435
##########
airflow/providers/neo4j/hooks/neo4j.py:
##########
@@ -91,12 +91,7 @@ def get_uri(self, conn: Connection) -> str:
elif trusted_ca:
encryption_scheme = '+s'
- return '{scheme}{encryption_scheme}://{host}:{port}'.format(
- scheme=scheme,
- encryption_scheme=encryption_scheme,
- host=conn.host,
- port='7687' if conn.port is None else f'{conn.port}',
- )
+ return f"{scheme}{encryption_scheme}://{conn.host}:{'7687' if conn.port is None else f'{conn.port}'}"
Review Comment:
```suggestion
return f"{scheme}{encryption_scheme}://{conn.host}:{7687 if conn.port is None else conn.port}"
```
--
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 #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #22887:
URL: https://github.com/apache/airflow/pull/22887#discussion_r849831169
##########
airflow/operators/subdag.py:
##########
@@ -115,14 +115,9 @@ def _validate_pool(self, session):
pool = session.query(Pool).filter(Pool.slots == 1).filter(Pool.pool == self.pool).first()
if pool and any(t.pool == self.pool for t in self.subdag.tasks):
raise AirflowException(
- 'SubDagOperator {sd} and subdag task{plural} {t} both '
- 'use pool {p}, but the pool only has 1 slot. The '
- 'subdag tasks will never run.'.format(
- sd=self.task_id,
- plural=len(conflicts) > 1,
- t=', '.join(t.task_id for t in conflicts),
- p=self.pool,
- )
+ f"SubDagOperator {self.task_id} and subdag task{len(conflicts) > 1} "
+ f"{', '.join(t.task_id for t in conflicts)} both use pool {self.pool}, "
+ f"but the pool only has 1 slot. The subdag tasks will never run."
Review Comment:
:) - we can like fix it in separate 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] github-actions[bot] commented on pull request #22887: Bump pre-commit hook versions
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22887:
URL: https://github.com/apache/airflow/pull/22887#issuecomment-1101245981
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