You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org> on 2026/02/07 11:06:10 UTC
[PR] UI: Show clear permission toast for 403 errors on user actions [airflow]
Abhishekmishra2808 opened a new pull request, #61588:
URL: https://github.com/apache/airflow/pull/61588
## Description
Fixes #61460
This PR improves the user experience when an action fails due to insufficient permissions.
A global React Query `MutationCache` error handler now intercepts HTTP **403 (Forbidden)** responses and displays a clear toast message:
> **Insufficient Permissions — You do not have permission to perform this action.**
The toast is deduplicated to prevent spam when multiple mutations fail simultaneously. Existing retry behavior and error propagation remain unchanged.
Scope is intentionally limited to **mutations (user actions)** to avoid noisy background query refresh toasts.
This provides clearer feedback for protected UI actions (such as triggering DAGs or editing resources) with minimal risk and no behavioral side effects.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2840935632
##########
airflow-core/src/airflow/ui/src/queries/useClearRun.ts:
##########
Review Comment:
I think we need to extract this into a utility function to not have this piece of code duplicated all over the place.
```ts
const createToaster = (error, titleKey, ...) => {}
```
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +41,33 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ?? (error as { response?: { status?: number } }).response?.status;
+
+ if (status === 403) {
+ // Only show one 403 toast at a time to prevent toast spam
+ // when multiple mutations fail simultaneously
+ if (active403ToastId === undefined || !toaster.isActive(active403ToastId)) {
+ active403ToastId = toaster.create({
+ description: i18n.t(
+ "errors.forbidden.description",
+ "You do not have permission to perform this action.",
Review Comment:
```suggestion
```
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-4010045730
@pierrejeambrun please check it 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
Re: [PR] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2959035471
##########
airflow-core/src/airflow/ui/src/queries/useDagParsing.ts:
##########
@@ -25,15 +25,15 @@ import {
UseDagSourceServiceGetDagSourceKeyFn,
} from "openapi/queries";
import { toaster } from "src/components/ui";
-import { createErrorToaster } from "src/components/ui/createErrorToaster";
+import { createErrorToaster } from "src/utils";
export const useDagParsing = ({ dagId }: { readonly dagId: string }) => {
const queryClient = useQueryClient();
- const { t: translate } = useTranslation(["dag", "common"]);
- const onError = createErrorToaster(translate, {
- description: translate("parse.toaster.error.description"),
- title: translate("parse.toaster.error.title"),
- });
+ const { t: translate } = useTranslation("dag");
+
+ const onError = (error: unknown) => {
+ createErrorToaster(error, { titleKey: "dag:parse.toaster.error.title" }, translate);
+ };
Review Comment:
Weird that the CI doesn't detect dead code
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-4103616402
Please check the PR now, I have squashed the commit into a single commit
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2840935632
##########
airflow-core/src/airflow/ui/src/queries/useClearRun.ts:
##########
Review Comment:
I think we need to extract this into a utility function to not have this piece of code duplicated all over the place. (from handling the error and skipping the toaster creation, to calling the 'toaster.create....`.
```ts
const createToaster = (error, titleKey, ...) => {}
```
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2891145808
##########
airflow-core/src/airflow/ui/src/queries/useDagParsing.ts:
##########
@@ -30,7 +30,16 @@ export const useDagParsing = ({ dagId }: { readonly dagId: string }) => {
const queryClient = useQueryClient();
const { t: translate } = useTranslation("dag");
- const onError = () => {
+ const onError = (error: unknown) => {
+ // Get status from error
+ const status =
+ (error as { status?: number }).status ?? (error as { response?: { status?: number } }).response?.status;
+
+ // Skip 403 errors as they are handled by MutationCache
Review Comment:
Can you reuse the 'createErrorToaster' here?
##########
airflow-core/src/airflow/ui/src/queries/useDeleteConnection.ts:
##########
@@ -27,6 +27,16 @@ export const useDeleteConnection = ({ onSuccessConfirm }: { onSuccessConfirm: ()
const { t: translate } = useTranslation(["admin", "common"]);
const onError = (error: Error) => {
+ // Get status from error
Review Comment:
Probably here too. etc... below other instances.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-4119908778
### Backport failed to create: v3-1-test. View the failure log <a href='https://github.com/apache/airflow/actions/runs/23502248514'> Run details </a>
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>❌</td>
<td>v3-1-test</td>
<td><a href="https://github.com/apache/airflow/commit/c8ad7a9851596c499c57839fdf306ef80bc7d1c6"><img src='https://img.shields.io/badge/Commit-c8ad7a9-red' alt='Commit Link'></a></td>
</tr>
</table>
You can attempt to backport this manually by running:
```bash
cherry_picker c8ad7a9 v3-1-test
```
This should apply the commit to the v3-1-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.
After you have resolved the conflicts, you can continue the backport process by running:
```bash
cherry_picker --continue
```
If you don't have cherry-picker installed, see the [installation guide](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#how-to-backport-pr-with-cherry-picker-cli).
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #61588:
URL: https://github.com/apache/airflow/pull/61588
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2897138118
##########
airflow-core/src/airflow/ui/src/queries/useDeleteDag.ts:
##########
@@ -34,6 +34,16 @@ export const useDeleteDag = ({
const { t: translate } = useTranslation();
const onError = (error: Error) => {
+ // Get status from error
+ const status =
+ (error as unknown as { status?: number }).status ??
+ (error as unknown as { response?: { status?: number } }).response?.status;
Review Comment:
@bbovenzi Nice Idea, Please check out the latest commit!
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2890822754
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +41,32 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ?? (error as { response?: { status?: number } }).response?.status;
+
+ if (status === 403) {
+ // Only show one 403 toast at a time to prevent toast spam
+ // when multiple mutations fail simultaneously
+ if (active403ToastId === undefined || !toaster.isActive(active403ToastId)) {
+ active403ToastId = toaster.create({
+ description: i18n.t(
Review Comment:
It looks like the static checks are failing here.
Could you please take a look?
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2822491077
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +40,31 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ??
+ (error as { response?: { status?: number } }).response?.status;
+
+ if (status === 403) {
+ // Only show one 403 toast at a time to prevent toast spam
+ // when multiple mutations fail simultaneously
+ if (active403ToastId === undefined || !toaster.isActive(active403ToastId)) {
+ active403ToastId = toaster.create({
+ description: "You do not have permission to perform this action.",
+ title: "Insufficient Permissions",
+ type: "error",
+ });
Review Comment:
I’ve updated the PR to use the translation system instead of hard-coded strings and centralized the 403 handling in MutationCache. Mutation-level toasters now skip 403 errors, so duplicates are no longer shown.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-3864697101
It would be helpful for the review if you could add a screenshot showing how the toast message looks😀
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-4018602797
addressed all the concern told by the reviewers. This PR is ready for the final review.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2894249883
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +41,32 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ?? (error as { response?: { status?: number } }).response?.status;
+
+ if (status === 403) {
+ // Only show one 403 toast at a time to prevent toast spam
+ // when multiple mutations fail simultaneously
+ if (active403ToastId === undefined || !toaster.isActive(active403ToastId)) {
+ active403ToastId = toaster.create({
+ description: i18n.t(
Review Comment:
now check ! CI turned green :-)
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2894247352
##########
airflow-core/src/airflow/ui/src/queries/useDeleteConnection.ts:
##########
@@ -27,6 +27,16 @@ export const useDeleteConnection = ({ onSuccessConfirm }: { onSuccessConfirm: ()
const { t: translate } = useTranslation(["admin", "common"]);
const onError = (error: Error) => {
+ // Get status from error
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
Re: [PR] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2958897443
##########
airflow-core/src/airflow/ui/src/queries/useClearTaskInstances.ts:
##########
@@ -52,6 +52,15 @@ export const useClearTaskInstances = ({
let detail: string;
let description: string;
+ // Get status from error
+ const status =
+ (error as { status?: number }).status ?? (error as { response?: { status?: number } }).response?.status;
+
+ // Skip 403 errors as they are handled by MutationCache
+ if (status === 403) {
+ return;
+ }
+
Review Comment:
`createErrorToaster` ?
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2799876909
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +40,31 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ??
+ (error as { response?: { status?: number } }).response?.status;
+
+ if (status === 403) {
+ // Only show one 403 toast at a time to prevent toast spam
+ // when multiple mutations fail simultaneously
+ if (active403ToastId === undefined || !toaster.isActive(active403ToastId)) {
+ active403ToastId = toaster.create({
+ description: "You do not have permission to perform this action.",
+ title: "Insufficient Permissions",
+ type: "error",
+ });
Review Comment:
https://github.com/apache/airflow/pull/61560 will take care of this and introduce a common 'handler' with existing toaster.
Then in this PR we can centralize all the calls to the toaster handler into this MutationCache we created.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2896856092
##########
airflow-core/src/airflow/ui/src/queries/useDeleteDag.ts:
##########
@@ -34,6 +34,16 @@ export const useDeleteDag = ({
const { t: translate } = useTranslation();
const onError = (error: Error) => {
+ // Get status from error
+ const status =
+ (error as unknown as { status?: number }).status ??
+ (error as unknown as { response?: { status?: number } }).response?.status;
Review Comment:
I feel like we should pull this out into a util instead of copy-pasting this code all over.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-3864671064
Static checks are failing, please take a look to setup local validations: https://github.com/apache/airflow/blob/main/contributing-docs/03a_contributors_quick_start_beginners.rst
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-4010580738
The i18n keys `errors.forbidden.description` and `errors.forbidden.title` seem to be missing from the translation files.
Could you please add them?
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-4119900885
🎉 Thanks @Abhishekmishra2808
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2822505263
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +40,31 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ??
+ (error as { response?: { status?: number } }).response?.status;
+
+ if (status === 403) {
+ // Only show one 403 toast at a time to prevent toast spam
+ // when multiple mutations fail simultaneously
+ if (active403ToastId === undefined || !toaster.isActive(active403ToastId)) {
+ active403ToastId = toaster.create({
Review Comment:
The 403 handling is already centralized in `MutationCache`, so it applies to all mutations returning 403, not just the case shown in the video. Other 403 errors will also display the global permission message.
This works independently but aligns with the common handler introduced in #61560.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2799611741
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +40,31 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ??
+ (error as { response?: { status?: number } }).response?.status;
+
+ if (status === 403) {
+ // Only show one 403 toast at a time to prevent toast spam
+ // when multiple mutations fail simultaneously
+ if (active403ToastId === undefined || !toaster.isActive(active403ToastId)) {
+ active403ToastId = toaster.create({
Review Comment:
When are we releasing the tracking? For other 403 errors to actually show a message? Example in the video above.
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +40,31 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ??
+ (error as { response?: { status?: number } }).response?.status;
+
+ if (status === 403) {
+ // Only show one 403 toast at a time to prevent toast spam
+ // when multiple mutations fail simultaneously
+ if (active403ToastId === undefined || !toaster.isActive(active403ToastId)) {
+ active403ToastId = toaster.create({
+ description: "You do not have permission to perform this action.",
+ title: "Insufficient Permissions",
+ type: "error",
+ });
Review Comment:
This can't be hard coded but needs a translation.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-3886954655
Instead of manually writing error messages in the UI, it would be better to make sure that fastapi is sending the correct error messages or that the UI is using the correct error message that is coming from 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
Re: [PR] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-4046475657
Common function was merged, can you re-use it in this PR? `createErrorToaster` from #61560
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2958897443
##########
airflow-core/src/airflow/ui/src/queries/useClearTaskInstances.ts:
##########
@@ -52,6 +52,15 @@ export const useClearTaskInstances = ({
let detail: string;
let description: string;
+ // Get status from error
+ const status =
+ (error as { status?: number }).status ?? (error as { response?: { status?: number } }).response?.status;
+
+ // Skip 403 errors as they are handled by MutationCache
+ if (status === 403) {
+ return;
+ }
+
Review Comment:
`createErrorToaster` ?
##########
airflow-core/src/airflow/ui/src/queries/useDagParsing.ts:
##########
@@ -25,15 +25,15 @@ import {
UseDagSourceServiceGetDagSourceKeyFn,
} from "openapi/queries";
import { toaster } from "src/components/ui";
-import { createErrorToaster } from "src/components/ui/createErrorToaster";
+import { createErrorToaster } from "src/utils";
export const useDagParsing = ({ dagId }: { readonly dagId: string }) => {
const queryClient = useQueryClient();
- const { t: translate } = useTranslation(["dag", "common"]);
- const onError = createErrorToaster(translate, {
- description: translate("parse.toaster.error.description"),
- title: translate("parse.toaster.error.title"),
- });
+ const { t: translate } = useTranslation("dag");
+
+ const onError = (error: unknown) => {
+ createErrorToaster(error, { titleKey: "dag:parse.toaster.error.title" }, translate);
+ };
Review Comment:
You need to delete `createErrorToaster` from `src/components/ui/createErrorToaster`, I think it was only used here.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-3920953800
Hi @pierrejeambrun,
I’ve addressed all the suggestions you shared. I’ve also attached a screenshot showing the updated behavior, the duplicate toaster issue is now resolved.
Please let me know if anything else needs improvement.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2799876909
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +40,31 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ??
+ (error as { response?: { status?: number } }).response?.status;
+
+ if (status === 403) {
+ // Only show one 403 toast at a time to prevent toast spam
+ // when multiple mutations fail simultaneously
+ if (active403ToastId === undefined || !toaster.isActive(active403ToastId)) {
+ active403ToastId = toaster.create({
+ description: "You do not have permission to perform this action.",
+ title: "Insufficient Permissions",
+ type: "error",
+ });
Review Comment:
https://github.com/apache/airflow/pull/61560 will take care of this and introduce a common 'handler' with existing toaster.
Then in this PR we can centralize all the calls to the toaster handler into this MutationCache we created. (And remove all of the manual toaster creation call we have everywhere)
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-4005667983
@pierrejeambrun Please re-run the CI and review this please. sorry for pinging you 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
Re: [PR] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2890822754
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +41,32 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ?? (error as { response?: { status?: number } }).response?.status;
+
+ if (status === 403) {
+ // Only show one 403 toast at a time to prevent toast spam
+ // when multiple mutations fail simultaneously
+ if (active403ToastId === undefined || !toaster.isActive(active403ToastId)) {
+ active403ToastId = toaster.create({
+ description: i18n.t(
Review Comment:
It looks like the static checks are failing here.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2911035918
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +41,30 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ?? (error as { response?: { status?: number } }).response?.status;
+
Review Comment:
Why not use getStatus? Same for other places.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-4063221310
@pierrejeambrun apologies for back and forth. Can you please review it 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
Re: [PR] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2936273288
##########
airflow-core/src/airflow/ui/src/queryClient.ts:
##########
@@ -39,6 +41,30 @@ const retryFunction = (failureCount: number, error: unknown) => {
return failureCount < RETRY_COUNT;
};
+// Track active 403 toast to prevent duplicates when multiple mutations fail
+let active403ToastId: string | undefined;
+
+// Error handler for 403 (Forbidden) responses on user-initiated actions
+const handle403Error = (error: unknown) => {
+ // Check for 403 (Forbidden) only to avoid interfering with 401 (Auth) logic
+ // Using nullish coalescing to safely find the status regardless of error shape
+ const status =
+ (error as { status?: number }).status ?? (error as { response?: { status?: number } }).response?.status;
+
Review Comment:
addressed in the latest commit.
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2840935632
##########
airflow-core/src/airflow/ui/src/queries/useClearRun.ts:
##########
Review Comment:
I think we need to extract this into a utility function to not have this piece of code duplicated all over the place. (from handling the error and skipping the toaster creation, to calling the 'toaster.create....`.
```ts
const createErrorToaster = (error, titleKey, ...) => {}
```
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2969741066
##########
airflow-core/src/airflow/ui/src/queries/useDagParsing.ts:
##########
@@ -25,15 +25,15 @@ import {
UseDagSourceServiceGetDagSourceKeyFn,
} from "openapi/queries";
import { toaster } from "src/components/ui";
-import { createErrorToaster } from "src/components/ui/createErrorToaster";
+import { createErrorToaster } from "src/utils";
export const useDagParsing = ({ dagId }: { readonly dagId: string }) => {
const queryClient = useQueryClient();
- const { t: translate } = useTranslation(["dag", "common"]);
- const onError = createErrorToaster(translate, {
- description: translate("parse.toaster.error.description"),
- title: translate("parse.toaster.error.title"),
- });
+ const { t: translate } = useTranslation("dag");
+
+ const onError = (error: unknown) => {
+ createErrorToaster(error, { titleKey: "dag:parse.toaster.error.title" }, translate);
+ };
Review Comment:
Accepted
--
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] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on PR #61588:
URL: https://github.com/apache/airflow/pull/61588#issuecomment-4010636556
@choo121600 added them in the latest commit, please check it 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
Re: [PR] UI: Show clear permission toast for 403 errors on user actions [airflow]
Posted by "Abhishekmishra2808 (via GitHub)" <gi...@apache.org>.
Abhishekmishra2808 commented on code in PR #61588:
URL: https://github.com/apache/airflow/pull/61588#discussion_r2894246733
##########
airflow-core/src/airflow/ui/src/queries/useDagParsing.ts:
##########
@@ -30,7 +30,16 @@ export const useDagParsing = ({ dagId }: { readonly dagId: string }) => {
const queryClient = useQueryClient();
const { t: translate } = useTranslation("dag");
- const onError = () => {
+ const onError = (error: unknown) => {
+ // Get status from error
+ const status =
+ (error as { status?: number }).status ?? (error as { response?: { status?: number } }).response?.status;
+
+ // Skip 403 errors as they are handled by MutationCache
Review Comment:
Yes, I have addressed in the latest commit!
--
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