-
Notifications
You must be signed in to change notification settings - Fork 15.1k
Implementing log_url
on RuntimeTaskInstance in task sdk
#50376
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
Conversation
@tirkarthi for review and guidence |
0e285f2
to
a591c6d
Compare
@amoghrajesh please review Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking ok, some comments.
providers/openlineage/src/airflow/providers/openlineage/utils/utils.py
Outdated
Show resolved
Hide resolved
43f57d1
to
d07bd3c
Compare
d969b1b
to
ea00cd9
Compare
log_url
on RuntimeTaskInstance in task sdk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM +1, thanks.
@mobuchowski are you ok with the OL side of things?
…ng taskrunner startup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me +1
@kacpermuda ok from OL side?
Yes @amoghrajesh , looks good, thanks ! |
Fixes #50330
The
log_url
variable is required in theemail.html
template. Since the configuration has been removed it is raising exception whenlog_url
andmark_success_url
is being used to send email notifications.This PR adds both to the task instance context
(context.ti)
during execution.In previous versions of Airflow, during execution
ti.log_url
was available,This PR introduces the following changes:
log_url
toRuntimeTaskInstance
log_url
property toti
inget_template_context
since context['ti'] needs to have log_url