Skip to content

Conversation

ayush3singh
Copy link
Contributor

@ayush3singh ayush3singh commented May 8, 2025

Fixes #50330

The log_url variable is required in the email.html template. Since the configuration has been removed it is raising exception when log_url and mark_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:

  • Adds log_url to RuntimeTaskInstance
  • Adds log_url property to ti in get_template_context since context['ti'] needs to have log_url

@boring-cyborg boring-cyborg bot added area:Executors-core LocalExecutor & SequentialExecutor area:task-sdk labels May 8, 2025
@ayush3singh
Copy link
Contributor Author

@tirkarthi for review and guidence

@ayush3singh ayush3singh force-pushed the issue_50330 branch 4 times, most recently from 0e285f2 to a591c6d Compare May 9, 2025 21:45
@ayush3singh ayush3singh marked this pull request as ready for review May 10, 2025 07:05
@ayush3singh
Copy link
Contributor Author

@amoghrajesh please review

Thanks

Copy link
Contributor

@amoghrajesh amoghrajesh left a 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.

@ayush3singh ayush3singh force-pushed the issue_50330 branch 3 times, most recently from 43f57d1 to d07bd3c Compare May 10, 2025 20:25
@ayush3singh ayush3singh force-pushed the issue_50330 branch 4 times, most recently from d969b1b to ea00cd9 Compare May 11, 2025 18:18
@amoghrajesh amoghrajesh changed the title Fixes issue RuntimeTaskInstance context does not contain log_url Implementing log_url on RuntimeTaskInstance in task sdk May 12, 2025
Copy link
Contributor

@amoghrajesh amoghrajesh left a 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?

Copy link
Contributor

@amoghrajesh amoghrajesh left a 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?

@kacpermuda
Copy link
Contributor

Yes @amoghrajesh , looks good, thanks !

@amoghrajesh amoghrajesh added this to the Airflow 3.1.0 milestone May 16, 2025
@amoghrajesh amoghrajesh merged commit 330789e into apache:main May 16, 2025
69 checks passed
@messense
Copy link
Member

I'm getting Failed to send notification: 'airflow.sdk.execution_time.task_runner.RuntimeTaskInstance object' has no attribute 'mark_success_url' even with this patch.

@amoghrajesh
Copy link
Contributor

CC @sunank200 do you want to take a stab at it?

@ayush3singh
Copy link
Contributor Author

@amoghrajesh
I can patch it since we removed the change as it was out of scope.
Is it fine if I raise a pr

@amoghrajesh
Copy link
Contributor

Oh ok, @ayush3singh feel free to create a new PR

@ayush3singh ayush3singh deleted the issue_50330 branch June 4, 2025 19:37
sanederchik pushed a commit to sanederchik/airflow that referenced this pull request Jun 7, 2025
@ashb ashb modified the milestones: Airflow 3.1.0, Airflow 3.0.5 Jul 9, 2025
kaxil pushed a commit that referenced this pull request Jul 9, 2025
@kaxil kaxil modified the milestones: Airflow 3.0.5, Airflow 3.0.3 Jul 9, 2025
kaxil pushed a commit that referenced this pull request Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Executors-core LocalExecutor & SequentialExecutor area:task-sdk
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RuntimeTaskInstance context does not contain log_url
7 participants