Skip to content

Conversation

@eumiro
Copy link
Contributor

@eumiro eumiro commented Aug 23, 2023

No description provided.

with DAG(
dag_id=Path(__file__).stem,
description=__doc__[0 : __doc__.find(".")],
description=__doc__.partition(".")[0],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not totally related but gosh I’m not sure we should encourage this pattern.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does have an appeal, especially if you consider the D400 rule of pydoc style.

Comment on lines +322 to +328
count_integrations = Counter(r.get("integration-name", "") for r in resource_data)
for integration, count in count_integrations.items():
if count > 1:
errors.append(
f"Duplicated content of '{resource_type}/integration-name/{integration}' "
f"in file: {yaml_file_path}"
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should just use ["integration-name"] instead of .get(). This would emit a nonsensical message when the key is missing anyway…

Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic-related changes mentioned above will be proposed in a separate PR for discussion.

@potiuk
Copy link
Member

potiuk commented Aug 24, 2023

Flaky test only - merging.

@potiuk potiuk merged commit 2dbb963 into apache:main Aug 24, 2023
@eumiro eumiro deleted the count-in-loop branch August 24, 2023 16:16
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.2 milestone Oct 2, 2023
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Oct 2, 2023
ephraimbuddy pushed a commit that referenced this pull request Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants