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 2021/01/18 04:57:46 UTC
[GitHub] [airflow] jmcarp opened a new pull request #13732: Switch to f-strings using flynt.
jmcarp opened a new pull request #13732:
URL: https://github.com/apache/airflow/pull/13732
Switch string formatting to f-strings consistently with `flynt`. We can also add `flynt` to ci if it would be useful. This will probably accumulate merge conflicts, but they should be easy to resolve.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-764349611
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 master 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-762073989
Something like that would do in `.pre-commit-config.yaml`:
```
- id: flynt
name: Convert to f-strings with flynt
entry: flynt
language: python
language_version: python3
require_serial: true
additional_dependencies: ['flynt']
files: \.py$
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jmcarp commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
jmcarp commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-765869346
Not sure what the ci failure is about. Is this safe to merge?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jmcarp commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
jmcarp commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-763175051
Ok, I added flynt to pre-commit and fixed nits from @XD-DENG.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-762071985
Nice! That would also help with cases like the failure above - where both flynt and black would cooperate via pre-commits :).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-762622326
[The Workflow run](https://github.com/apache/airflow/actions/runs/495280622) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-762073989
Something like that would do in .pre-commit:
```
- id: flynt
name: Format f-strings with flynt
entry: flynt
language: python
language_version: python3
require_serial: true
additional_dependencies: ['flynt']
files: \.py$
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-762000090
[The Workflow run](https://github.com/apache/airflow/actions/runs/492843211) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jmcarp commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
jmcarp commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-764263944
Thanks, should be good to merge with an approval from @XD-DENG!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-764349611
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #13732:
URL: https://github.com/apache/airflow/pull/13732
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-762073989
Something like that would do in `.pre-commit-config.yaml`:
```
- id: flynt
name: Format f-strings with flynt
entry: flynt
language: python
language_version: python3
require_serial: true
additional_dependencies: ['flynt']
files: \.py$
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-762073989
Something like that would do in .pre-commit:
```
- id: flynt
name: Format f-strings with flynt
entry: flynt
args: [--fail-on-change]
language: python
language_version: python3
require_serial: true
additional_dependencies: ['flynt']
files: \.py$
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-765013763
[The Workflow run](https://github.com/apache/airflow/actions/runs/502383580) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jmcarp commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
jmcarp commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-764263944
Thanks, should be good to merge with an approval from @XD-DENG!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-765869699
Looks unrelated (and likely random GitHub failure). Merging!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-762404210
[The Workflow run](https://github.com/apache/airflow/actions/runs/494116553) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] XD-DENG commented on a change in pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#discussion_r559458200
##########
File path: airflow/models/dag.py
##########
@@ -1538,7 +1538,7 @@ def pickle_info(self):
dttm = timezone.utcnow()
pickled = pickle.dumps(self)
d['pickle_len'] = len(pickled)
- d['pickling_duration'] = "{}".format(timezone.utcnow() - dttm)
+ d['pickling_duration'] = f"{timezone.utcnow() - dttm}"
Review comment:
Here using `str(timezone.utcnow() - dttm})` may make more sense than f"{timezone.utcnow() - dttm}" IMO, given it's purely a type conversion rather than formating.
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -166,7 +166,7 @@ def set_is_paused(is_paused, args):
is_paused=is_paused,
)
- print("Dag: {}, paused: {}".format(args.dag_id, str(is_paused)))
+ print(f"Dag: {args.dag_id}, paused: {str(is_paused)}")
Review comment:
nit: the `str()` can be removed?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-764963062
[The Workflow run](https://github.com/apache/airflow/actions/runs/502117295) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-764887997
[The Workflow run](https://github.com/apache/airflow/actions/runs/501553247) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #13732: Switch to f-strings using flynt.
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #13732:
URL: https://github.com/apache/airflow/pull/13732#issuecomment-762073989
Something like that would do in `.pre-commit-config.yaml`:
```
- id: flynt
name: Convert to f-strings with flynt
entry: flynt
language: python
language_version: python3
additional_dependencies: ['flynt']
files: \.py$
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org