Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor shorter defaults #34350

Merged
merged 1 commit into from Oct 21, 2023
Merged

Refactor shorter defaults #34350

merged 1 commit into from Oct 21, 2023

Conversation

eumiro
Copy link
Contributor

@eumiro eumiro commented Sep 13, 2023

No description provided.

Comment on lines 57 to 58
execution_date = execution_date if execution_date else timezone.utcnow()
if execution_date is None:
execution_date = timezone.utcnow()
Copy link
Member

Choose a reason for hiding this comment

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

Would execution_date = execution_date or timezone.utcnow() work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. However, while other cases are c = a or b and we need to attribute something to c, this is a = a or b, so if a is trueish, it is not modified.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah and I don’t think that’s a problem for this particular case.

@vincbeck
Copy link
Contributor

Conflicts

Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

CI Failures are from main, otherwise changes are clean. LGTM!

@potiuk
Copy link
Member

potiuk commented Oct 21, 2023

Rebased to see if it works with latest main.

@eladkal eladkal merged commit 7d80b7d into apache:main Oct 21, 2023
62 checks passed
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Oct 27, 2023
@ephraimbuddy ephraimbuddy added this to the Airflow 2.8.0 milestone Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API area:CLI area:core-operators Operators, Sensors and hooks within Core Airflow area:dev-tools area:providers area:Scheduler Scheduler or dag parsing Issues area:system-tests area:Triggerer area:webserver Webserver related Issues provider:amazon-aws AWS/Amazon - related issues provider:cncf-kubernetes Kubernetes provider related issues provider:dbt-cloud provider:docker provider:jenkins provider:microsoft-azure Azure-related issues type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants