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/09/21 20:36:02 UTC
[GitHub] [airflow] pierrejeambrun opened a new pull request, #26575: Fix 404 taskInstance errors and validateDOMNesting warnings
pierrejeambrun opened a new pull request, #26575:
URL: https://github.com/apache/airflow/pull/26575
We had a few warnings due to `validateDOMNesting` errors in the grid view (cf screenshot) -> split the table into two distinct tables with separate 'titles'. Took the opportunity to slightly improve spacing between the tables.
We also had a few 404 errors when requesting the taskInstance for groups (cf screenshot):
--
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] pierrejeambrun commented on pull request #26575: Grid, fix 404 taskInstance errors
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #26575:
URL: https://github.com/apache/airflow/pull/26575#issuecomment-1255232930
> I like your approach of splitting it into 2 tables more. When you rebase your branch. Let's keep that.
Ok, i did that and updated the titles so we have something consistent with the 'Task Actions' title:
--
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] pierrejeambrun commented on pull request #26575: Grid, fix 404 taskInstance errors and validateDOMNesting warnings
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #26575:
URL: https://github.com/apache/airflow/pull/26575#issuecomment-1254637557
> code looks fine but it's nice to include a screenshot of the visual change
You are right. I didn't change much so I didn't bother, but here is a preview:
--
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] pierrejeambrun commented on a diff in pull request #26575: Grid, fix 404 taskInstance errors
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #26575:
URL: https://github.com/apache/airflow/pull/26575#discussion_r977832875
##########
airflow/www/static/js/dag/details/taskInstance/Details.tsx:
##########
@@ -57,7 +58,7 @@ const Details = ({ instance, group, dagId }: Props) => {
} = instance;
const { data: apiTI } = useTaskInstance({
- dagId, dagRunId: runId, taskId, mapIndex, enabled: true,
+ dagId, dagRunId: runId, taskId, mapIndex, enabled: !isGroup,
Review Comment:
True the same problem appears for mapped task. Fixed.
--
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 #26575: Grid, fix 404 taskInstance errors
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #26575:
URL: https://github.com/apache/airflow/pull/26575#discussion_r977586008
##########
airflow/www/static/js/dag/details/taskInstance/Details.tsx:
##########
@@ -57,7 +58,7 @@ const Details = ({ instance, group, dagId }: Props) => {
} = instance;
const { data: apiTI } = useTaskInstance({
- dagId, dagRunId: runId, taskId, mapIndex, enabled: true,
+ dagId, dagRunId: runId, taskId, mapIndex, enabled: !isGroup,
Review Comment:
We may need to checked for `isMapped` too
##########
airflow/www/static/js/dag/details/taskInstance/Details.tsx:
##########
@@ -57,7 +58,7 @@ const Details = ({ instance, group, dagId }: Props) => {
} = instance;
const { data: apiTI } = useTaskInstance({
- dagId, dagRunId: runId, taskId, mapIndex, enabled: true,
+ dagId, dagRunId: runId, taskId, mapIndex, enabled: !isGroup,
Review Comment:
We may need to check for `isMapped` too
--
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] pierrejeambrun commented on a diff in pull request #26575: Grid, fix 404 taskInstance errors
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #26575:
URL: https://github.com/apache/airflow/pull/26575#discussion_r977832875
##########
airflow/www/static/js/dag/details/taskInstance/Details.tsx:
##########
@@ -57,7 +58,7 @@ const Details = ({ instance, group, dagId }: Props) => {
} = instance;
const { data: apiTI } = useTaskInstance({
- dagId, dagRunId: runId, taskId, mapIndex, enabled: true,
+ dagId, dagRunId: runId, taskId, mapIndex, enabled: !isGroup,
Review Comment:
True the same problem appears also for mapped task. Fixed.
--
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] dstandish commented on pull request #26575: Grid, fix 404 taskInstance errors
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #26575:
URL: https://github.com/apache/airflow/pull/26575#issuecomment-1255369690
nice, thanks, yeah i knew the formatting was off but could not figure out what was wrong ... this looks great...
--
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 pull request #26575: Grid, fix 404 taskInstance errors
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on PR #26575:
URL: https://github.com/apache/airflow/pull/26575#issuecomment-1254943888
I like your approach of splitting it into 2 tables more. When you rebase your branch. Let's keep that.
--
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 merged pull request #26575: Grid, fix 404 taskInstance errors
Posted by GitBox <gi...@apache.org>.
bbovenzi merged PR #26575:
URL: https://github.com/apache/airflow/pull/26575
--
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