You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/08/03 05:25:31 UTC
[GitHub] [airflow] uranusjr opened a new pull request, #25500: Make extra link work in UI
uranusjr opened a new pull request, #25500:
URL: https://github.com/apache/airflow/pull/25500
Two parts in this:
1. The existing extra links mechanism relies on the operator being unmapped, so we do that for most operators to keep working.
2. The UI needs to be slightly updated to ask for those links at the right time -- namely, it must only make requests against unmapped tasks, not the mapped ones.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr merged pull request #25500: Make extra link work in UI
Posted by GitBox <gi...@apache.org>.
uranusjr merged PR #25500:
URL: https://github.com/apache/airflow/pull/25500
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #25500: Make extra link work in UI
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #25500:
URL: https://github.com/apache/airflow/pull/25500#issuecomment-1203892306
> The UI needs to be updated to ask for those links at the right time -- namely, it must only make requests against unmapped tasks, not the mapped ones.
Hmmm. Ideally It would be nice if each individual mapped task could have it's own mapped links too. For example if your mapped task submits jobs to EMR/Databricks etc then being able to have mapped links to the remote logs/job is desirable.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #25500: Make extra link work in UI
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #25500:
URL: https://github.com/apache/airflow/pull/25500#issuecomment-1203924011
> Hmmm. Ideally It would be nice if each individual mapped task could have it's own mapped links too. For example if your mapped task submits jobs to EMR/Databricks etc then being able to have mapped links to the remote logs/job is desirable.
Yes, this is what this PR intends to do. What we’re currently doing is completely the other way around: we display the links against the entire mapped task (and that doesn’t work well for most things), but not for individual (mapped) task instances. This PR removes the former, and adds the entries for the latter.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #25500: Make extra link work in UI
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25500:
URL: https://github.com/apache/airflow/pull/25500#discussion_r936248159
##########
airflow/models/abstractoperator.py:
##########
@@ -401,8 +414,8 @@ def render_template(
template = jinja_env.from_string(value)
dag = self.get_dag()
if dag and dag.render_template_as_native_obj:
- return render_template_as_native(template, context)
- return render_template_to_string(template, context)
+ return render_template_as_native(template, cast(MutableMapping[str, Any], context))
+ return render_template_to_string(template, cast(MutableMapping[str, Any], context))
Review Comment:
Unrelated, but Mypy is complaining. `context` here is _implemented_ as a MutableMapping (although we supply a type stub to say it’s not to prevent users from getting a wrong idea), so this is fine.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi commented on a diff in pull request #25500: Make extra link work in UI
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25500:
URL: https://github.com/apache/airflow/pull/25500#discussion_r936397794
##########
airflow/www/static/js/dag.js:
##########
@@ -269,7 +271,7 @@ export function callModal({
}
query.delete('try_number');
- if (extraLinks && extraLinks.length > 0) {
+ if (!isMapped && extraLinks && extraLinks.length > 0) {
Review Comment:
Should we also wrap this in a `!isMapped`?https://github.com/apache/airflow/blob/main/airflow/www/static/js/dag/details/taskInstance/index.tsx#L167
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #25500: Make extra link work in UI
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25500:
URL: https://github.com/apache/airflow/pull/25500#discussion_r936407128
##########
airflow/www/static/js/dag.js:
##########
@@ -269,7 +271,7 @@ export function callModal({
}
query.delete('try_number');
- if (extraLinks && extraLinks.length > 0) {
+ if (!isMapped && extraLinks && extraLinks.length > 0) {
Review Comment:
I guess, I only fixed the view mentioned in #25360 because that’s the part I can understand :p
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #25500: Make extra link work in UI
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25500:
URL: https://github.com/apache/airflow/pull/25500#discussion_r936248159
##########
airflow/models/abstractoperator.py:
##########
@@ -401,8 +414,8 @@ def render_template(
template = jinja_env.from_string(value)
dag = self.get_dag()
if dag and dag.render_template_as_native_obj:
- return render_template_as_native(template, context)
- return render_template_to_string(template, context)
+ return render_template_as_native(template, cast(MutableMapping[str, Any], context))
+ return render_template_to_string(template, cast(MutableMapping[str, Any], context))
Review Comment:
Unrelated, but Mypy is complaining.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org