You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bbovenzi (via GitHub)" <gi...@apache.org> on 2025/01/31 16:49:51 UTC
[PR] Add autorefresh to dags list page [airflow]
bbovenzi opened a new pull request, #46326:
URL: https://github.com/apache/airflow/pull/46326
Add auto refresh to dags list.
Replace the isFetching progress bar with a spinner on each refreshing dag.
Update the dag details query cache instead of refreshing on toggling is_paused
<img width="1197" alt="Screenshot 2025-01-31 at 11 29 35 AM" src="https://github.com/user-attachments/assets/b5e4bdaa-57cb-4ba2-9d34-d0ed1e0c50b1" />
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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
Re: [PR] Add autorefresh to dags list page [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #46326:
URL: https://github.com/apache/airflow/pull/46326#discussion_r1937641504
##########
airflow/ui/src/components/TogglePause.tsx:
##########
@@ -42,12 +43,19 @@ export const TogglePause = ({ dagDisplayName, dagId, isPaused, skipConfirm }: Pr
const { onClose, onOpen, open } = useDisclosure();
const onSuccess = async () => {
- await queryClient.invalidateQueries({
- queryKey: [useDagServiceGetDagsKey],
- });
+ // We can just update dag details
+ queryClient.setQueryData<DAGDetailsResponse>(UseDagServiceGetDagDetailsKeyFn({ dagId }), (oldData) =>
+ oldData
+ ? {
+ ...oldData,
+ is_paused: !oldData.is_paused,
+ }
+ : undefined,
+ );
Review Comment:
I'm not super fan of this.
If we add some business logic and side effect on pausing / unpausing a dag. That will start failing.
We are unnecessary coupling the back and the front on that part. I think an extra GET is really not that expensive and worth the trouble. (if we generalize that pattern we might end up with issues)
--
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
Re: [PR] Add autorefresh to dags list page [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #46326:
URL: https://github.com/apache/airflow/pull/46326#discussion_r1937641504
##########
airflow/ui/src/components/TogglePause.tsx:
##########
@@ -42,12 +43,19 @@ export const TogglePause = ({ dagDisplayName, dagId, isPaused, skipConfirm }: Pr
const { onClose, onOpen, open } = useDisclosure();
const onSuccess = async () => {
- await queryClient.invalidateQueries({
- queryKey: [useDagServiceGetDagsKey],
- });
+ // We can just update dag details
+ queryClient.setQueryData<DAGDetailsResponse>(UseDagServiceGetDagDetailsKeyFn({ dagId }), (oldData) =>
+ oldData
+ ? {
+ ...oldData,
+ is_paused: !oldData.is_paused,
+ }
+ : undefined,
+ );
Review Comment:
I'm not super fan of this.
If we add some business logic and side effect on pausing / unpausing a dag in the backend, this will start failing. (data will be de-synced)
We are unnecessary coupling the back and the front on that part. I think an extra GET is really not that expensive and worth the trouble. (if we generalize that pattern we might end up with problem, or extra maintenance cost. As long as performance is not an issue, I wouldn't try to optimize this)
--
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
Re: [PR] Add autorefresh to dags list page [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #46326:
URL: https://github.com/apache/airflow/pull/46326#discussion_r1937641504
##########
airflow/ui/src/components/TogglePause.tsx:
##########
@@ -42,12 +43,19 @@ export const TogglePause = ({ dagDisplayName, dagId, isPaused, skipConfirm }: Pr
const { onClose, onOpen, open } = useDisclosure();
const onSuccess = async () => {
- await queryClient.invalidateQueries({
- queryKey: [useDagServiceGetDagsKey],
- });
+ // We can just update dag details
+ queryClient.setQueryData<DAGDetailsResponse>(UseDagServiceGetDagDetailsKeyFn({ dagId }), (oldData) =>
+ oldData
+ ? {
+ ...oldData,
+ is_paused: !oldData.is_paused,
+ }
+ : undefined,
+ );
Review Comment:
I'm not super fan of this.
If we add some business logic and side effect on pausing / unpausing a dag in the backend, this will start failing.
We are unnecessary coupling the back and the front on that part. I think an extra GET is really not that expensive and worth the trouble. (if we generalize that pattern we might end up with issues)
##########
airflow/ui/src/components/TogglePause.tsx:
##########
@@ -42,12 +43,19 @@ export const TogglePause = ({ dagDisplayName, dagId, isPaused, skipConfirm }: Pr
const { onClose, onOpen, open } = useDisclosure();
const onSuccess = async () => {
- await queryClient.invalidateQueries({
- queryKey: [useDagServiceGetDagsKey],
- });
+ // We can just update dag details
+ queryClient.setQueryData<DAGDetailsResponse>(UseDagServiceGetDagDetailsKeyFn({ dagId }), (oldData) =>
+ oldData
+ ? {
+ ...oldData,
+ is_paused: !oldData.is_paused,
+ }
+ : undefined,
+ );
Review Comment:
I'm not super fan of this.
If we add some business logic and side effect on pausing / unpausing a dag in the backend, this will start failing. (data will be de-synced)
We are unnecessary coupling the back and the front on that part. I think an extra GET is really not that expensive and worth the trouble. (if we generalize that pattern we might end up with issues)
--
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
Re: [PR] Add autorefresh to dags list page [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46326:
URL: https://github.com/apache/airflow/pull/46326#discussion_r1937658152
##########
airflow/ui/src/components/TogglePause.tsx:
##########
@@ -42,12 +43,19 @@ export const TogglePause = ({ dagDisplayName, dagId, isPaused, skipConfirm }: Pr
const { onClose, onOpen, open } = useDisclosure();
const onSuccess = async () => {
- await queryClient.invalidateQueries({
- queryKey: [useDagServiceGetDagsKey],
- });
+ // We can just update dag details
+ queryClient.setQueryData<DAGDetailsResponse>(UseDagServiceGetDagDetailsKeyFn({ dagId }), (oldData) =>
+ oldData
+ ? {
+ ...oldData,
+ is_paused: !oldData.is_paused,
+ }
+ : undefined,
+ );
Review Comment:
Ok I'll drop 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
Re: [PR] Add autorefresh to dags list page [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi merged PR #46326:
URL: https://github.com/apache/airflow/pull/46326
--
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