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/08 17:10:28 UTC
[GitHub] [airflow] pierrejeambrun opened a new pull request, #25610: Grid logs for mapped instances
pierrejeambrun opened a new pull request, #25610:
URL: https://github.com/apache/airflow/pull/25610
https://github.com/apache/airflow/pull/25568 added support for retrieving mapped instance task logs.
This PR adds support for displaying these logs in the Grid details side panel.
- Mapped Instances rows in the table are not selectable anymore. (Actions are done on all of them)
- Mapped Instances rows are clickable and bring a new details page for mapped instances. (We can see the Map Index on top of other detailed information).
- For the new details, logs tab is available and show the logs for this particular map index.
- Breadcrumbs get extended with the map index and the `taskInstance` breadcrumbs becomes clickable in this particular case.
- Extracting tasks actions into its own component.
- Added a few unit tests.
Tested on:
- Mapped Tasks (obviously) :heavy_check_mark:
- Groups with mapped task :heavy_check_mark:
- Logs display, `See More` and `Download` links, and logs filters :heavy_check_mark:
- When there is multiple pages in the `MappedInstances` Table :heavy_check_mark:
- Breadcrumbs navigation :heavy_check_mark:
- Task actions for all mapped task are working :heavy_check_mark:
- Task actions from the details of a mapped task instance does not work with the local executor but the calls seems fine. (I didn't modify that part)
_Notes: This adds logic to the `TaskInstance` which is already quite complicated. I believe it went from 'complexe' to 'too complexe', but I do not see a trivial way to refactor it. I do not want to add a lot of changes on top of that PR, I'll most certainly open a follow up PR to this end. Let me know what you think about 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 commented on a diff in pull request #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r942904951
##########
airflow/www/static/js/dag/details/taskInstance/index.tsx:
##########
@@ -54,18 +52,41 @@ const dagId = getMetaValue('dag_id')!;
interface Props {
taskId: Task['id'];
runId: DagRun['runId'];
+ mapIndex: TaskInstanceType['mapIndex'];
+ onSelect: (selectionProps: SelectionProps) => void;
}
-const TaskInstance = ({ taskId, runId }: Props) => {
- const [selectedRows, setSelectedRows] = useState<number[]>([]);
+const TaskInstance = ({
+ taskId, runId, mapIndex, onSelect,
+}: Props) => {
+ const isMapIndexDefined = !(mapIndex === undefined);
+ const selectedRows: number[] = isMapIndexDefined ? [mapIndex] : [];
const { data: { dagRuns, groups } } = useGridData();
const storageTabIndex = parseInt(localStorage.getItem(detailsPanelActiveTabIndex) || '0', 10);
const [preferedTabIndex, setPreferedTabIndex] = useState(storageTabIndex);
+ const [instance, setInstance] = useState<TaskInstanceType>();
+
const group = getTask({ taskId, task: groups });
const run = dagRuns.find((r) => r.runId === runId);
+ useEffect(() => {
+ if (!isMapIndexDefined) {
+ setInstance(group?.instances.find((ti) => ti.runId === runId));
Review Comment:
By using a new API endpoint for a single mapped instance, we can remove a lot of this `useEffect` logic, add autorefresh, and make sure that navigating to this page with `map_index` in the URl, data will load.
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r942895417
##########
airflow/www/static/js/dag/details/taskInstance/index.tsx:
##########
@@ -54,18 +52,41 @@ const dagId = getMetaValue('dag_id')!;
interface Props {
taskId: Task['id'];
runId: DagRun['runId'];
+ mapIndex: TaskInstanceType['mapIndex'];
+ onSelect: (selectionProps: SelectionProps) => void;
}
-const TaskInstance = ({ taskId, runId }: Props) => {
- const [selectedRows, setSelectedRows] = useState<number[]>([]);
+const TaskInstance = ({
+ taskId, runId, mapIndex, onSelect,
+}: Props) => {
+ const isMapIndexDefined = !(mapIndex === undefined);
+ const selectedRows: number[] = isMapIndexDefined ? [mapIndex] : [];
const { data: { dagRuns, groups } } = useGridData();
const storageTabIndex = parseInt(localStorage.getItem(detailsPanelActiveTabIndex) || '0', 10);
const [preferedTabIndex, setPreferedTabIndex] = useState(storageTabIndex);
+ const [instance, setInstance] = useState<TaskInstanceType>();
+
const group = getTask({ taskId, task: groups });
const run = dagRuns.find((r) => r.runId === runId);
+ useEffect(() => {
+ if (!isMapIndexDefined) {
+ setInstance(group?.instances.find((ti) => ti.runId === runId));
Review Comment:
I don't think we should be using `useEffect` here. The Details component should probably get the mapped instance info from the API: `/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/{map_index}`
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25610:
URL: https://github.com/apache/airflow/pull/25610#issuecomment-1209793600
I updated the PR and the description.
- Nav link is done, and removed them from the table, nice suggestion. :)
I'm not sure about direct log access, I feel like it is already covered by the preferred tab. If the user preferred tab is 'logs" he will directly land on logs by clicking on the table row otherwise he will land on details.
- Added the button to go back to the dynamic task summary, good idea!
- Separate the mapped task table on a new tab only available for mapped task summary. Unfortunately I do not have the number of mapped instance info available atm. (this could be an improvement).
`Logs` & `Mapped Tasks` tab are mutually exclusive. In terms of 'preferredTab' they act as the same. Meaning you can go from 'mapped' -> 'logs' and reverse without changing tabs.
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r945294291
##########
airflow/www/static/js/dag/useSelection.test.tsx:
##########
@@ -38,17 +38,19 @@ describe('Test useSelection hook', () => {
selected: {
runId,
taskId,
+ mapIndex,
},
} = result.current;
expect(runId).toBeNull();
expect(taskId).toBeNull();
+ expect(mapIndex).toBeNull();
});
test.each([
- { taskId: 'task_1', runId: 'run_1' },
- { runId: 'run_1', taskId: null },
- { taskId: 'task_1', runId: null },
+ { taskId: 'task_1', runId: 'run_1', mapIndex: 2 },
+ { runId: 'run_1', taskId: null, mapIndex: null },
+ { taskId: 'task_1', runId: null, mapIndex: 1 },
Review Comment:
Done :)
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r942895948
##########
airflow/www/static/js/dag/details/taskInstance/index.tsx:
##########
@@ -54,18 +52,41 @@ const dagId = getMetaValue('dag_id')!;
interface Props {
taskId: Task['id'];
runId: DagRun['runId'];
+ mapIndex: TaskInstanceType['mapIndex'];
+ onSelect: (selectionProps: SelectionProps) => void;
}
-const TaskInstance = ({ taskId, runId }: Props) => {
- const [selectedRows, setSelectedRows] = useState<number[]>([]);
+const TaskInstance = ({
+ taskId, runId, mapIndex, onSelect,
+}: Props) => {
+ const isMapIndexDefined = !(mapIndex === undefined);
+ const selectedRows: number[] = isMapIndexDefined ? [mapIndex] : [];
Review Comment:
Let's rename it.
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r942889264
##########
airflow/www/static/js/dag/details/taskInstance/index.tsx:
##########
@@ -54,18 +52,41 @@ const dagId = getMetaValue('dag_id')!;
interface Props {
taskId: Task['id'];
runId: DagRun['runId'];
+ mapIndex: TaskInstanceType['mapIndex'];
+ onSelect: (selectionProps: SelectionProps) => void;
}
-const TaskInstance = ({ taskId, runId }: Props) => {
- const [selectedRows, setSelectedRows] = useState<number[]>([]);
+const TaskInstance = ({
+ taskId, runId, mapIndex, onSelect,
+}: Props) => {
+ const isMapIndexDefined = !(mapIndex === undefined);
+ const selectedRows: number[] = isMapIndexDefined ? [mapIndex] : [];
Review Comment:
SelectedRow is currently used for the task actions. (Empty <=> all mapped, when non empty only run on those mapIndex).
The name is misleading indeed
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r942860590
##########
airflow/www/static/js/dag/details/taskInstance/BackToTaskSummary.tsx:
##########
@@ -0,0 +1,50 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React from 'react';
+import { Button, Flex } from '@chakra-ui/react';
+
+interface Props {
+ isMapIndexDefined: boolean;
+ onClick: () => void;
+}
+
+const BackToTaskSummary = ({ isMapIndexDefined, onClick }: Props) => {
+ if (!isMapIndexDefined) return null;
+
+ return (
+ <div>
+ {isMapIndexDefined && (
Review Comment:
Since we have the `return null` above, we can remove the div and conditional block
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25610:
URL: https://github.com/apache/airflow/pull/25610#issuecomment-1211316741
> I don't think we should be using `useEffect` here. The Details component should probably get the mapped instance info from the API: `/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/{map_index}`
True this would simplify the logic.
Converted to draft while I implement the suggested change to the API.
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r942881603
##########
airflow/www/static/js/dag/details/taskInstance/index.tsx:
##########
@@ -54,18 +52,41 @@ const dagId = getMetaValue('dag_id')!;
interface Props {
taskId: Task['id'];
runId: DagRun['runId'];
+ mapIndex: TaskInstanceType['mapIndex'];
+ onSelect: (selectionProps: SelectionProps) => void;
}
-const TaskInstance = ({ taskId, runId }: Props) => {
- const [selectedRows, setSelectedRows] = useState<number[]>([]);
+const TaskInstance = ({
+ taskId, runId, mapIndex, onSelect,
+}: Props) => {
+ const isMapIndexDefined = !(mapIndex === undefined);
+ const selectedRows: number[] = isMapIndexDefined ? [mapIndex] : [];
Review Comment:
Do we need selectedRows anymore? We got rid of multiselect.
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r942908863
##########
airflow/www/static/js/dag/details/taskInstance/BackToTaskSummary.tsx:
##########
@@ -0,0 +1,50 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React from 'react';
+import { Button, Flex } from '@chakra-ui/react';
+
+interface Props {
+ isMapIndexDefined: boolean;
+ onClick: () => void;
+}
+
+const BackToTaskSummary = ({ isMapIndexDefined, onClick }: Props) => {
+ if (!isMapIndexDefined) return null;
+
+ return (
+ <div>
+ {isMapIndexDefined && (
Review Comment:
done
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r945159218
##########
airflow/www/static/js/dag/useSelection.test.tsx:
##########
@@ -38,17 +38,19 @@ describe('Test useSelection hook', () => {
selected: {
runId,
taskId,
+ mapIndex,
},
} = result.current;
expect(runId).toBeNull();
expect(taskId).toBeNull();
+ expect(mapIndex).toBeNull();
});
test.each([
- { taskId: 'task_1', runId: 'run_1' },
- { runId: 'run_1', taskId: null },
- { taskId: 'task_1', runId: null },
+ { taskId: 'task_1', runId: 'run_1', mapIndex: 2 },
+ { runId: 'run_1', taskId: null, mapIndex: null },
+ { taskId: 'task_1', runId: null, mapIndex: 1 },
Review Comment:
Let's add a test case for `mapIndex: 0` and `mapIndex: -1`.
For -1, we probably need to cast it to null. Currently, I don't see anywhere this pops up in the UI, but a test couldn't hurt.
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25610:
URL: https://github.com/apache/airflow/pull/25610#issuecomment-1211275835
> Since we store the map_index in the url, we need to make sure we load an individual mapped task instance details. Try refreshing the page when you have a single mapped task selected and the details panel will be blank.
I broke that in my last commit :facepalm:...
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r944743036
##########
airflow/www/static/js/dag/details/taskInstance/BackToTaskSummary.tsx:
##########
@@ -0,0 +1,47 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React from 'react';
+import { Button, Flex } from '@chakra-ui/react';
+
+interface Props {
+ isMapIndexDefined: boolean;
+ onClick: () => void;
+}
+
+const BackToTaskSummary = ({ isMapIndexDefined, onClick }: Props) => {
+ if (!isMapIndexDefined) return null;
+
+ return (
+ <div>
Review Comment:
removed
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
bbovenzi merged PR #25610:
URL: https://github.com/apache/airflow/pull/25610
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on PR #25610:
URL: https://github.com/apache/airflow/pull/25610#issuecomment-1211209044
Since we store the map_index in the url, we need to make sure we load an individual mapped task instance details.
Try refreshing the page when you have a single mapped task selected and the details panel will be blank.
Also, a task instance might not have logs (ie: it is scheduled but hasn't run yet). We should handle that case instead of just showing a blank page to a user.
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r942910409
##########
airflow/www/static/js/dag/details/taskInstance/index.tsx:
##########
@@ -54,18 +52,41 @@ const dagId = getMetaValue('dag_id')!;
interface Props {
taskId: Task['id'];
runId: DagRun['runId'];
+ mapIndex: TaskInstanceType['mapIndex'];
+ onSelect: (selectionProps: SelectionProps) => void;
}
-const TaskInstance = ({ taskId, runId }: Props) => {
- const [selectedRows, setSelectedRows] = useState<number[]>([]);
+const TaskInstance = ({
+ taskId, runId, mapIndex, onSelect,
+}: Props) => {
+ const isMapIndexDefined = !(mapIndex === undefined);
+ const selectedRows: number[] = isMapIndexDefined ? [mapIndex] : [];
Review Comment:
done
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25610:
URL: https://github.com/apache/airflow/pull/25610#issuecomment-1211280366
Maybe this is the source of all the confusion.
I checked the autorefresh for mapped task, it is working now.
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25610:
URL: https://github.com/apache/airflow/pull/25610#issuecomment-1214827527
Thank you Brent :)
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25610:
URL: https://github.com/apache/airflow/pull/25610#discussion_r944677127
##########
airflow/www/static/js/dag/details/taskInstance/BackToTaskSummary.tsx:
##########
@@ -0,0 +1,47 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React from 'react';
+import { Button, Flex } from '@chakra-ui/react';
+
+interface Props {
+ isMapIndexDefined: boolean;
+ onClick: () => void;
+}
+
+const BackToTaskSummary = ({ isMapIndexDefined, onClick }: Props) => {
+ if (!isMapIndexDefined) return null;
+
+ return (
+ <div>
Review Comment:
Looks like the `<div>` got added back again.
--
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 #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25610:
URL: https://github.com/apache/airflow/pull/25610#issuecomment-1213486336
With your suggestions I was able to come up with a much simpler implementation, that I feel will be more robust as well.
Thanks
--
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 closed pull request #25610: Grid logs for mapped instances
Posted by GitBox <gi...@apache.org>.
pierrejeambrun closed pull request #25610: Grid logs for mapped instances
URL: https://github.com/apache/airflow/pull/25610
--
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