jscheffl commented on code in PR #55277:
URL: https://github.com/apache/airflow/pull/55277#discussion_r2323505722
##########
airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGButton.tsx:
##########
@@ -31,9 +33,20 @@ type Props = {
readonly withText?: boolean;
};
+const TRIGGER_QUERY_KEY = "trigger";
+const isTruthy = (value: string | null) => value === "true" || value === "1";
+
const TriggerDAGButton: React.FC<Props> = ({ dagDisplayName, dagId, isPaused,
withText = true }) => {
const { onClose, onOpen, open } = useDisclosure();
const { t: translate } = useTranslation("components");
+ const { search } = useLocation();
+ const params = useMemo(() => new URLSearchParams(search), [search]);
+
+ useEffect(() => {
Review Comment:
The usage of `useEffect()` is restricted and should only be used when there
is no other alternative.
##########
airflow-core/src/airflow/ui/src/utils/query.ts:
##########
@@ -47,3 +47,45 @@ export const useAutoRefresh = ({ dagId, isPaused }: {
dagId?: string; isPaused?:
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
return (canRefresh ? autoRefreshInterval * 1000 : false) as number | false;
};
+
+export const decodeParam = (params: URLSearchParams, key: string): string |
null => {
+ const value = params.get(key);
+
+ if (value === null) {
+ return null;
+ }
+ try {
+ return decodeURIComponent(value);
+ } catch {
+ // Return original value if decoding fails
+ return value;
+ }
+};
+
+export const parseJsonSafe = (raw: string | null): string | null => {
+ if (raw === null) {
+ return null;
+ }
+ try {
+ const parsed: unknown = JSON.parse(raw);
+
+ return JSON.stringify(parsed, null, 2);
+ } catch {
+ // Return original string if parsing fails
+ return raw;
Review Comment:
Why do you return the original if parsing fails? Then it would not be "safe"
and some follow-up code will fail on this?
##########
airflow-core/src/airflow/ui/src/constants/reprocessBehaviourParams.ts:
##########
@@ -16,8 +16,23 @@
* specific language governing permissions and limitations
* under the License.
*/
+import type { ReprocessBehavior } from "openapi/requests/types.gen";
+
export const reprocessBehaviors = [
{ label: "backfill.missingRuns", value: "none" },
{ label: "backfill.missingAndErroredRuns", value: "failed" },
{ label: "backfill.allRuns", value: "completed" },
];
+
+export const mapReprocessBehavior = (param: string): ReprocessBehavior => {
Review Comment:
Why do you map the strings from the API to the user URL? Would it not be
easier to use the same in the URL as well? Just probably needs some
documentation so that users know about it.
##########
airflow-core/src/airflow/ui/src/constants/reprocessBehaviourParams.ts:
##########
@@ -16,8 +16,23 @@
* specific language governing permissions and limitations
* under the License.
*/
+import type { ReprocessBehavior } from "openapi/requests/types.gen";
+
export const reprocessBehaviors = [
{ label: "backfill.missingRuns", value: "none" },
{ label: "backfill.missingAndErroredRuns", value: "failed" },
{ label: "backfill.allRuns", value: "completed" },
];
+
+export const mapReprocessBehavior = (param: string): ReprocessBehavior => {
+ switch (param) {
+ case "all_runs":
+ return "completed";
+ case "missing_and_errored_runs":
+ return "failed";
+ case "missing_runs":
+ return "none";
+ default:
+ return "none";
Review Comment:
Default can be used for the missing as well
```suggestion
default:
return "none";
```
##########
airflow-core/src/airflow/ui/src/utils/query.ts:
##########
@@ -47,3 +47,45 @@ export const useAutoRefresh = ({ dagId, isPaused }: {
dagId?: string; isPaused?:
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
return (canRefresh ? autoRefreshInterval * 1000 : false) as number | false;
};
+
+export const decodeParam = (params: URLSearchParams, key: string): string |
null => {
+ const value = params.get(key);
+
+ if (value === null) {
+ return null;
+ }
+ try {
+ return decodeURIComponent(value);
+ } catch {
+ // Return original value if decoding fails
+ return value;
Review Comment:
Same as below - why returning the original string if decoding fails? This
would allow passing "bad" data further into later code and makes it very hard
to understand bugs?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
