uranusjr commented on a change in pull request #19099:
URL: https://github.com/apache/airflow/pull/19099#discussion_r732724241
##########
File path: airflow/models/dagcode.py
##########
@@ -96,7 +96,7 @@ def bulk_sync_to_db(cls, filelocs: Iterable[str],
session=None):
for fileloc in conflicting_filelocs:
message += (
"Filename '{}' causes a hash collision in the "
- + "database with '{}'. Please rename the file."
+ "database with '{}'. Please rename the file."
).format(hashes_to_filelocs[DagCode.dag_fileloc_hash(fileloc)], fileloc)
Review comment:
Can we also change this to an f-string?
##########
File path: airflow/providers/docker/example_dags/example_docker_copy_data.py
##########
@@ -75,7 +75,7 @@
"/bin/bash",
"-c",
"/bin/sleep 30; "
- "/bin/mv {{ params.source_location }}/" + f"{t_view.output}" + " {{
params.target_location }};"
+ "/bin/mv {{ params.source_location }}/" + str(t_view.output) + " {{
params.target_location }};"
Review comment:
Please definitely use an f-string here.
##########
File path: airflow/operators/google_api_to_s3_transfer.py
##########
@@ -43,7 +43,7 @@ def __init__(self, **kwargs):
"""This class is deprecated.
Please use
`airflow.providers.amazon.aws.transfers."""
- + "google_api_to_s3_transfer.GoogleApiToS3Operator`.",
+ "google_api_to_s3_transfer.GoogleApiToS3Operator`.",
Review comment:
I wonder if it's a mistake to use multi-line strings here in the first
place. This warning message format is not present anywhere else as far as I
know (and you can see the module deprecation message just above doesn't add
these newlines).
##########
File path: tests/providers/amazon/aws/utils/eks_test_constants.py
##########
@@ -198,38 +198,41 @@ class RegExTemplates:
"""The compiled RegEx patterns used in testing."""
CLUSTER_ARN: Pattern = re.compile(
- "arn:"
- + "(?P<partition>.+):"
- + "eks:"
- + "(?P<region>[-0-9a-zA-Z]+):"
- + "(?P<account_id>[0-9]{12}):"
- + "cluster/"
- + "(?P<cluster_name>.+)"
+ r"""arn:
+ (?P<partition>.+):
+ eks:
+ (?P<region>[-0-9a-zA-Z]+):
+ (?P<account_id>[0-9]{12}):
+ cluster/
+ (?P<cluster_name>.+)""",
+ re.VERBOSE,
)
FARGATE_PROFILE_ARN: Pattern = re.compile(
- "arn:"
- + "(?P<partition>.+):"
- + "eks:"
- + "(?P<region>[-0-9a-zA-Z]+):"
- + "(?P<account_id>[0-9]{12}):"
- + "fargateprofile/"
- + "(?P<cluster_name>.+)/"
- + "(?P<fargate_name>.+)/"
- + FARGATE_PROFILE_UUID_PATTERN
+ r"""arn:
+ (?P<partition>.+):
+ eks:
+ (?P<region>[-0-9a-zA-Z]+):
+ (?P<account_id>[0-9]{12}):
+ fargateprofile/
+ (?P<cluster_name>.+)/
+ (?P<fargate_name>.+)/"""
+ + FARGATE_PROFILE_UUID_PATTERN,
+ re.VERBOSE,
)
NODEGROUP_ARN: Pattern = re.compile(
- "arn:"
- + "(?P<partition>.+):"
- + "eks:"
- + "(?P<region>[-0-9a-zA-Z]+):"
- + "(?P<account_id>[0-9]{12}):"
- + "nodegroup/"
- + "(?P<cluster_name>.+)/"
- + "(?P<nodegroup_name>.+)/"
- + NODEGROUP_UUID_PATTERN
+ r"""arn:
+ (?P<partition>.+):
+ eks:
+ (?P<region>[-0-9a-zA-Z]+):
+ (?P<account_id>[0-9]{12}):
+ nodegroup/
+ (?P<cluster_name>.+)/
+ (?P<nodegroup_name>.+)/"""
+ + NODEGROUP_UUID_PATTERN,
+ re.VERBOSE,
)
- NODEGROUP_ASG_NAME_PATTERN: Pattern = re.compile("eks-" +
NODEGROUP_UUID_PATTERN)
- NODEGROUP_SECURITY_GROUP_NAME_PATTERN: Pattern = re.compile("sg-" +
"([-0-9a-z]{17})")
+ NODEGROUP_ASG_NAME_PATTERN: Pattern =
re.compile(f"eks-{NODEGROUP_UUID_PATTERN}")
+ NODEGROUP_SECURITY_GROUP_NAME_PATTERN: Pattern =
re.compile("sg-([-0-9a-z]{17})")
Review comment:
Add an `r` here would improve readability a bit (syntax highlighting).
##########
File path: airflow/providers/databricks/hooks/databricks.py
##########
@@ -216,7 +216,7 @@ def _do_api_call(self, endpoint_info, json):
if attempt_num == self.retry_limit:
raise AirflowException(
- ('API requests to Databricks failed {} times. ' + 'Giving
up.').format(self.retry_limit)
+ ('API requests to Databricks failed {} times. Giving
up.').format(self.retry_limit)
Review comment:
Also f-string?
--
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]
