Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
jscheffl commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3046529653 As expressed earlier... Cool! -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bugraoz93 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3046365375 Great job! Congrats! I have been traveling so I couldn't 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3045737539 🎉 🎉 🎉 -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
boring-cyborg[bot] commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3045735421 Awesome work, congrats on your first merged pull request! You are invited to check our [Issue Tracker](https://github.com/apache/airflow/issues) for additional contributions. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3045735958 Great job! #protm -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun merged PR #51264: URL: https://github.com/apache/airflow/pull/51264 -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3045438655 > There's a conflict on a generated file (because something must have been merged in main). > > We need one more rebase and regeneration of front-end types and we should be good to merge. > > ```shell > pre-commit run ts-compile-format-lint-ui --all-files > ``` Hello @pierrejeambrun, I just rebased and regenerated the frontend types with **pre-commit run ts-compile-lint-ui --all-files**. Let me know if everything is as expected. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3039148163 Hello @bugraoz93, I did what you suggested and with the rebase, I think the hash generation problem is solved (at least the breeze static-checks --all-files command doesn't fail the test). Also, the MySQL problem should be fixed, but let me know if I did miss anything. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bugraoz93 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3036779376 Could you please try to build your CI image again and clean your environment, and try again? You can find all those related commands from here, and additionally, the breeze is showing in the outputs how to clean, etc... https://github.com/apache/airflow/tree/main/dev/breeze/doc -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3036556387 Hello @pierrejeambrun @bbovenzi @bugraoz93, I pushed some changes that should resolve the issues in the CI pipeline. I used the commands **breeze static-checks --all-files** and **breeze testing providers-tests --run-in-parallel --run-db-tests-only**, to verify the changes locally. However, I'm not sure why the static check error related to the hash generation was happening. Also, I don't understand why when I executed the **pre-commit run** it did change the **providers/fab/www-hash.txt**. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bugraoz93 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3032852378 Looks good! Great addition, thanks for the changes! Addition to Pierre's point on MySQL foreign key error. We need to fix the static check too related to hash generation -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3032549507 Looks like the migration is not working on mysql and needs to be adjusted. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3028324977 Hello @bbovenzi @pierrejeambrun, I just pushed some changes that should resolve the errors that were occurring in the CI pipeline. Also, I did a rebase to solve a conflict, but the changes should be present in the last 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bbovenzi commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3025641286 Looks like the CI is failing the same way even with the rebase -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3025187386 Hello @pierrejeambrun @bbovenzi, I just rebased and added the condition to clean the table for dag favorite. Let me know if everything is as expected. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3024433560 > Looks like the codegen API files are being formatted when they shouldn't be Might need to rebase we're pretty behind main. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3024296302 #protm -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3019251779 Hello @pierrejeambrun, I just pushed some changes that should resolve the issues you pointed out. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2175131105
##
airflow-core/src/airflow/ui/src/pages/Dag/Header.tsx:
##
@@ -115,7 +116,10 @@ export const Header = ({
stats={stats}
subTitle={
dag !== undefined && (
-
+
+
+
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2175129034
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDagCard.tsx:
##
@@ -0,0 +1,56 @@
+/*!
+ * 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 { Box, Button, Text, VStack } from "@chakra-ui/react";
+import { Link as RouterLink } from "react-router-dom";
+
+import type { DAGRunResponse } from "openapi/requests/types.gen";
+import { RecentRuns } from "src/pages/DagsList/RecentRuns";
+
+type FavoriteDagProps = {
+ readonly dagId: string;
+ readonly dagName: string;
+ readonly latestRuns: Array;
+};
+
+export const FavoriteDagCard = ({ dagId, dagName, latestRuns }:
FavoriteDagProps) => (
+
+
+
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2175128724
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDagCard.tsx:
##
@@ -0,0 +1,56 @@
+/*!
+ * 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 { Box, Button, Text, VStack } from "@chakra-ui/react";
+import { Link as RouterLink } from "react-router-dom";
+
+import type { DAGRunResponse } from "openapi/requests/types.gen";
+import { RecentRuns } from "src/pages/DagsList/RecentRuns";
+
+type FavoriteDagProps = {
+ readonly dagId: string;
+ readonly dagName: string;
+ readonly latestRuns: Array;
+};
+
+export const FavoriteDagCard = ({ dagId, dagName, latestRuns }:
FavoriteDagProps) => (
+
+
+ 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 { Box, Button, Text, VStack } from "@chakra-ui/react";
+import { Link as RouterLink } from "react-router-dom";
+
+import type { DAGRunResponse } from "openapi/requests/types.gen";
+import { RecentRuns } from "src/pages/DagsList/RecentRuns";
+
+type FavoriteDagProps = {
+ readonly dagId: string;
+ readonly dagName: string;
+ readonly latestRuns: Array;
+};
+
+export const FavoriteDagCard = ({ dagId, dagName, latestRuns }:
FavoriteDagProps) => (
+
+
+
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2175126060
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDagCard.tsx:
##
@@ -0,0 +1,56 @@
+/*!
+ * 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 { Box, Button, Text, VStack } from "@chakra-ui/react";
+import { Link as RouterLink } from "react-router-dom";
+
+import type { DAGRunResponse } from "openapi/requests/types.gen";
+import { RecentRuns } from "src/pages/DagsList/RecentRuns";
+
+type FavoriteDagProps = {
+ readonly dagId: string;
+ readonly dagName: string;
+ readonly latestRuns: Array;
+};
+
+export const FavoriteDagCard = ({ dagId, dagName, latestRuns }:
FavoriteDagProps) => (
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bbovenzi commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2167588890
##
airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.tsx:
##
@@ -132,6 +133,12 @@ const createColumns = (
enableSorting: false,
header: "",
},
+ {
+accessorKey: "favorite",
+cell: ({ row: { original } }) => ,
Review Comment:
```suggestion
accessorKey: "favorite",
cell: ({ row: { original } }) => ,
enableHiding: false,
```
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bbovenzi commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2167584284
##
airflow-core/src/airflow/ui/src/queries/useFavoriteDag.ts:
##
@@ -0,0 +1,48 @@
+/*!
+ * 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 { useQueryClient } from "@tanstack/react-query";
+
+import {
+ UseDagRunServiceGetDagRunsKeyFn,
+ UseDagServiceGetDagDetailsKeyFn,
+ UseDagServiceGetDagKeyFn,
+ useDagServiceGetDagsUiKey,
+ useDagServiceFavoriteDag,
+ UseTaskInstanceServiceGetTaskInstancesKeyFn,
+} from "openapi/queries";
+
+export const useFavoriteDag = ({ dagId }: { dagId: string }) => {
+ const queryClient = useQueryClient();
+
+ const onSuccess = async () => {
+const queryKeys = [
+ [useDagServiceGetDagsUiKey],
+ UseDagServiceGetDagKeyFn({ dagId }, [{ dagId }]),
+ UseDagServiceGetDagDetailsKeyFn({ dagId }, [{ dagId }]),
+ UseDagRunServiceGetDagRunsKeyFn({ dagId }, [{ dagId }]),
+ UseTaskInstanceServiceGetTaskInstancesKeyFn({ dagId, dagRunId: "~" }, [{
dagId, dagRunId: "~" }]),
Review Comment:
I think we actually only need to invalidate the the first key, no? The rest
are ok.
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bbovenzi commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2167582151
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,73 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useQueryClient } from "@tanstack/react-query";
+import { useEffect } from "react";
+import { useTranslation } from "react-i18next";
+import { FiStar } from "react-icons/fi";
+import { useLocation } from "react-router-dom";
+
+import { useDagServiceGetDagsUi } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { t: translate } = useTranslation("dashboard");
+ const LIMIT = 10;
+ const { data: favorites } = useDagServiceGetDagsUi({ isFavorite: true,
limit: LIMIT });
+
+ useEffect(() => {
+void queryClient.refetchQueries({ queryKey: ["DagServiceGetDagsUi"] });
+ }, [location.key, queryClient]);
Review Comment:
What are you trying to do 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2164045609
##
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##
@@ -112,6 +114,35 @@ def depends(cls, offset: NonNegativeInt = 0) ->
OffsetFilter:
return cls().set_value(offset)
+class _FavoriteFilter(BaseParam[bool]):
+"""Filter DAGs by favorite status."""
+
+user_id: str
+
+def to_orm(self, select_stmt: Select) -> Select:
+if self.value is None and self.skip_none:
+return select_stmt
+
+if self.value:
+select_stmt = select_stmt.join(DagFavorite, DagFavorite.dag_id ==
DagModel.dag_id).where(
+DagFavorite.user_id == self.user_id
+)
+else:
+select_stmt = select_stmt.where(
+~select(DagFavorite)
+.where((DagFavorite.dag_id == DagModel.dag_id) &
(DagFavorite.user_id == self.user_id))
+.exists()
+)
Review Comment:
Please don't use bitwise operator, sqlalchemy exposes "not" "and" "or" which
are clearer.
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,72 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useQueryClient } from "@tanstack/react-query";
+import { useEffect } from "react";
+import { useTranslation } from "react-i18next";
+import { FiStar } from "react-icons/fi";
+import { useLocation } from "react-router-dom";
+
+import { useDagServiceGetDagsUi } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { t: translate } = useTranslation("dashboard");
+ const { data: favorites } = useDagServiceGetDagsUi({ isFavorite: true,
limit: 10 });
+
+ useEffect(() => {
+void queryClient.refetchQueries({ queryKey: ["DagServiceGetDagsUi"] });
+ }, [location.key, queryClient]);
+
+ if (!favorites) {
+return undefined;
+ }
+
+ return (
+
+
+
+
+ {translate("favorite.favoriteDags")}
Review Comment:
Translation need to be changed to `First 10 favorite dags" or something like
this. To explicit the fact that not all of the favorite dags will be displayed
there but just the first 10 .
##
airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml:
##
@@ -3140,6 +3148,104 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/HTTPValidationError'
+ /api/v2/dags/{dag_id}/favorite:
+post:
+ tags:
+ - DAG
+ summary: Favorite Dag
+ description: Mark the DAG as favorite.
+ operationId: favorite_dag
+ security:
+ - OAuth2PasswordBearer: []
+ parameters:
+ - name: dag_id
+in: path
+required: true
+schema:
+ type: string
+ title: Dag Id
+ responses:
+'200':
+ description: Successful Response
+ content:
+application/json:
+ schema: {}
Review Comment:
Spec is wrong that should be `204`
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -317,6 +322,64 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="GET")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> Response:
+"""Mark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(insert(DagFavorite).values(dag_id=dag_id, user_id=user_id))
+
+return Response(status_code=status.HTTP_204_NO_CONTENT)
+
Review Comment:
Please check at other endpoints to
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264: URL: https://github.com/apache/airflow/pull/51264#discussion_r2167378597 ## airflow-core/src/airflow/api_fastapi/common/parameters.py: ## @@ -112,6 +114,35 @@ def depends(cls, offset: NonNegativeInt = 0) -> OffsetFilter: return cls().set_value(offset) +class _FavoriteFilter(BaseParam[bool]): +"""Filter DAGs by favorite status.""" + +user_id: str + +def to_orm(self, select_stmt: Select) -> Select: +if self.value is None and self.skip_none: +return select_stmt + +if self.value: +select_stmt = select_stmt.join(DagFavorite, DagFavorite.dag_id == DagModel.dag_id).where( +DagFavorite.user_id == self.user_id +) +else: +select_stmt = select_stmt.where( +~select(DagFavorite) +.where((DagFavorite.dag_id == DagModel.dag_id) & (DagFavorite.user_id == self.user_id)) +.exists() +) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-3005794354 Hello @pierrejeambrun, Your requests should be fixed. Let me know if everything is as expected. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2167380564
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,72 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useQueryClient } from "@tanstack/react-query";
+import { useEffect } from "react";
+import { useTranslation } from "react-i18next";
+import { FiStar } from "react-icons/fi";
+import { useLocation } from "react-router-dom";
+
+import { useDagServiceGetDagsUi } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { t: translate } = useTranslation("dashboard");
+ const { data: favorites } = useDagServiceGetDagsUi({ isFavorite: true,
limit: 10 });
+
+ useEffect(() => {
+void queryClient.refetchQueries({ queryKey: ["DagServiceGetDagsUi"] });
+ }, [location.key, queryClient]);
+
+ if (!favorites) {
+return undefined;
+ }
+
+ return (
+
+
+
+
+ {translate("favorite.favoriteDags")}
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2167379214
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -317,6 +322,64 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="GET")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> Response:
+"""Mark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(insert(DagFavorite).values(dag_id=dag_id, user_id=user_id))
+
+return Response(status_code=status.HTTP_204_NO_CONTENT)
+
Review Comment:
Done.
##
airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml:
##
@@ -3140,6 +3148,104 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/HTTPValidationError'
+ /api/v2/dags/{dag_id}/favorite:
+post:
+ tags:
+ - DAG
+ summary: Favorite Dag
+ description: Mark the DAG as favorite.
+ operationId: favorite_dag
+ security:
+ - OAuth2PasswordBearer: []
+ parameters:
+ - name: dag_id
+in: path
+required: true
+schema:
+ type: string
+ title: Dag Id
+ responses:
+'200':
+ description: Successful Response
+ content:
+application/json:
+ schema: {}
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2164077614
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,72 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useQueryClient } from "@tanstack/react-query";
+import { useEffect } from "react";
+import { useTranslation } from "react-i18next";
+import { FiStar } from "react-icons/fi";
+import { useLocation } from "react-router-dom";
+
+import { useDagServiceGetDagsUi } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { t: translate } = useTranslation("dashboard");
+ const { data: favorites } = useDagServiceGetDagsUi({ isFavorite: true,
limit: 10 });
+
+ useEffect(() => {
+void queryClient.refetchQueries({ queryKey: ["DagServiceGetDagsUi"] });
+ }, [location.key, queryClient]);
+
+ if (!favorites) {
+return undefined;
+ }
+
+ return (
+
+
+
+
+ {translate("favorite.favoriteDags")}
Review Comment:
Translation need to be changed to `First 10 favorite dags" or something like
this. To explicit the fact that not all of the favorite dags will be displayed
there but just the first 10 . (You can pass `limit` value as a translation
parameter)
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2164051515
##
airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml:
##
@@ -3140,6 +3148,104 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/HTTPValidationError'
+ /api/v2/dags/{dag_id}/favorite:
+post:
+ tags:
+ - DAG
+ summary: Favorite Dag
+ description: Mark the DAG as favorite.
+ operationId: favorite_dag
+ security:
+ - OAuth2PasswordBearer: []
+ parameters:
+ - name: dag_id
+in: path
+required: true
+schema:
+ type: string
+ title: Dag Id
+ responses:
+'200':
+ description: Successful Response
+ content:
+application/json:
+ schema: {}
Review Comment:
Spec is wrong that should be `204` (but if you follow how we do it for 204
no response, this will be corrected)
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-2994402049 Hello @pierrejeambrun, I just pushed a new commit. All your requests should be solved, but let me know if I missed anything. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2160444610
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,96 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useQueryClient } from "@tanstack/react-query";
+import { useEffect, useMemo, useState } from "react";
+import { useTranslation } from "react-i18next";
+import { FiStar } from "react-icons/fi";
+import { useLocation } from "react-router-dom";
+
+import { useDagServiceGetDags } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+
+const MAX_VISIBLE = 5;
Review Comment:
Done.
##
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dags.py:
##
@@ -392,6 +420,90 @@ def test_patch_dags_should_response_403(self,
unauthorized_test_client):
assert response.status_code == 403
+class TestFavoriteDag(TestDagEndpoint):
+"""Unit tests for favoriting a DAG."""
+
[email protected](
+"dag_id, expected_status_code, expected_exist_in_favorites",
+[
+("fake_dag_id", 404, None),
+(DAG1_ID, 200, True),
+],
+)
+def test_favorite_dag(
+self, test_client, dag_id, expected_status_code,
expected_exist_in_favorites, session
+):
+response = test_client.post(f"/dags/{dag_id}/favorite")
+assert response.status_code == expected_status_code
+
+if expected_status_code == 200:
+result = session.execute(
+select(DagFavorite).where(DagFavorite.dag_id == dag_id,
DagFavorite.user_id == "test")
+).first()
+assert result is not None if expected_exist_in_favorites else
result is None
+check_last_log(session, dag_id=dag_id, event="favorite_dag",
logical_date=None)
+
+def test_favorite_dag_should_response_401(self,
unauthenticated_test_client):
+response =
unauthenticated_test_client.post(f"/dags/{DAG1_ID}/favorite")
+assert response.status_code == 401
+
+def test_favorite_dag_should_response_403(self, unauthorized_test_client):
+response = unauthorized_test_client.post(f"/dags/{DAG1_ID}/favorite")
+assert response.status_code == 403
+
+def test_favoriting_already_favorited_dag_returns_409(self, test_client):
+response = test_client.post(f"/dags/{DAG1_ID}/favorite")
+assert response.status_code == 200
+
+response = test_client.post(f"/dags/{DAG1_ID}/favorite")
+assert response.status_code == 409
+
+
+class TestUnfavoriteDag(TestDagEndpoint):
+"""Unit tests for unfavoriting a DAG."""
+
[email protected](
+"dag_id, expected_status_code, expected_exist_in_favorites",
+[
+("fake_dag_id", 404, None),
+(DAG1_ID, 200, False),
+],
+)
+def test_unfavorite_dag(
+self, test_client, dag_id, expected_status_code,
expected_exist_in_favorites, session
+):
+if dag_id != "fake_dag_id":
+session.execute(insert(DagFavorite).values(dag_id=dag_id,
user_id="test"))
+session.commit()
+
+response = test_client.post(f"/dags/{dag_id}/unfavorite")
+assert response.status_code == expected_status_code
+
+if expected_status_code == 200:
+result = session.execute(
+select(DagFavorite).where(DagFavorite.dag_id == dag_id,
DagFavorite.user_id == "test")
+).first()
+assert result is not None if expected_exist_in_favorites else
result is None
+check_last_log(session, dag_id=dag_id, event="unfavorite_dag",
logical_date=None)
+
+def test_unfavorite_dag_should_response_401(self,
unauthenticated_test_client):
+response =
unauthenticated_test_client.post(f"/dags/{DAG1_ID}/unfavorite")
+assert response.status_code == 401
+
+def test_unfavorite_dag_should_response_403(self,
unauthorized_test_client):
+response = unauthorized_test_client.post(f"/dags/{DAG1_ID}/unfavorite")
+assert response.status_co
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2160442972
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +358,55 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Mark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(insert(DagFavorite).values(dag_id=dag_id, user_id=user_id))
+session.commit()
+
+return DAGResponse.model_validate(dag)
+
+
+@dags_router.post(
+"/{dag_id}/unfavorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
+)
+def unfavorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Unmark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(
+delete(DagFavorite).where(
+DagFavorite.dag_id == dag_id,
+DagFavorite.user_id == user_id,
+)
+)
+session.commit()
+
+return DAGResponse.model_validate(dag)
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2160445095
##
airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.tsx:
##
@@ -33,6 +33,7 @@ import { useLocalStorage } from "usehooks-ts";
import type { DagRunState, DAGWithLatestDagRunsResponse } from
"openapi/requests/types.gen";
import DeleteDagButton from "src/components/DagActions/DeleteDagButton";
+import { FavoriteDagButton } from
"src/components/DagActions/FavoriteDagButton";
Review Comment:
Done.
For this I made a filter very similar to the pause filter (with the options
all, favorite and unfavorite).
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2160443629
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -113,9 +116,12 @@ def get_dags(
],
readable_dags_filter: ReadableDagsFilterDep,
session: SessionDep,
+user: GetUserDep,
+is_favorite: QueryFavoriteFilter,
) -> DAGCollectionResponse:
"""Get all DAGs."""
-query = select(DagModel)
+is_favorite.user_id = user.get_id()
+query = is_favorite.to_orm(select(DagModel))
Review Comment:
Done.
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +352,64 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="GET")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Mark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(insert(DagFavorite).values(dag_id=dag_id, user_id=user_id))
+
+return DAGResponse.model_validate(dag)
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2160444517
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,96 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useQueryClient } from "@tanstack/react-query";
+import { useEffect, useMemo, useState } from "react";
+import { useTranslation } from "react-i18next";
+import { FiStar } from "react-icons/fi";
+import { useLocation } from "react-router-dom";
+
+import { useDagServiceGetDags } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+
+const MAX_VISIBLE = 5;
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { t: translate } = useTranslation("dashboard");
+ const { data: favorites } = useDagServiceGetDags({ isFavorite: true });
Review Comment:
All done, except the follow-up.
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264: URL: https://github.com/apache/airflow/pull/51264#discussion_r2160443575 ## airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py: ## @@ -113,9 +116,12 @@ def get_dags( ], readable_dags_filter: ReadableDagsFilterDep, session: SessionDep, +user: GetUserDep, +is_favorite: QueryFavoriteFilter, ) -> DAGCollectionResponse: """Get all DAGs.""" -query = select(DagModel) +is_favorite.user_id = user.get_id() 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2160443130
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,96 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useQueryClient } from "@tanstack/react-query";
+import { useEffect, useMemo, useState } from "react";
+import { useTranslation } from "react-i18next";
+import { FiStar } from "react-icons/fi";
+import { useLocation } from "react-router-dom";
+
+import { useDagServiceGetDags } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+
+const MAX_VISIBLE = 5;
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { t: translate } = useTranslation("dashboard");
+ const { data: favorites } = useDagServiceGetDags({ favorites: true });
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2159084356
##
airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.tsx:
##
@@ -33,6 +33,7 @@ import { useLocalStorage } from "usehooks-ts";
import type { DagRunState, DAGWithLatestDagRunsResponse } from
"openapi/requests/types.gen";
import DeleteDagButton from "src/components/DagActions/DeleteDagButton";
+import { FavoriteDagButton } from
"src/components/DagActions/FavoriteDagButton";
Review Comment:
We also probably need a new 'filter' in case the user only want to see, list
and filter his favorite dags. (A button to toggle/untoggle filter on
`is_favorite True/False`)
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2159068035
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,96 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useQueryClient } from "@tanstack/react-query";
+import { useEffect, useMemo, useState } from "react";
+import { useTranslation } from "react-i18next";
+import { FiStar } from "react-icons/fi";
+import { useLocation } from "react-router-dom";
+
+import { useDagServiceGetDags } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+
+const MAX_VISIBLE = 5;
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { t: translate } = useTranslation("dashboard");
+ const { data: favorites } = useDagServiceGetDags({ isFavorite: true });
Review Comment:
The UI needs to fetch dags with `useDagServiceGetDagsUi`. All other places
where we need that are using the UI version. (and things are cached). (This is
why you also need to propagate your backend change to the `ui` endpoint. `def
get_dags` in the `ui/dags.py` file.
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -113,9 +116,12 @@ def get_dags(
],
readable_dags_filter: ReadableDagsFilterDep,
session: SessionDep,
+user: GetUserDep,
+is_favorite: QueryFavoriteFilter,
) -> DAGCollectionResponse:
"""Get all DAGs."""
-query = select(DagModel)
+is_favorite.user_id = user.get_id()
+query = is_favorite.to_orm(select(DagModel))
Review Comment:
Why do we perform a manual call to the `to_orm` parameter?
Passing the filter to `paginated_select` should do exactly that.
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +352,64 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="GET")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Mark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(insert(DagFavorite).values(dag_id=dag_id, user_id=user_id))
+
+return DAGResponse.model_validate(dag)
Review Comment:
lets try no content 204.
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +358,55 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Mark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(insert(DagFavorite).values(dag_id=dag_id, user_id=user_id))
+session.commit()
+
+return DAGResponse.model_validate(dag)
+
+
+@dags_router.post(
+"/{dag_id}/unfavorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
+)
+def unfavorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Unmark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(
+delete(DagFavor
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-2985693353 Hello @pierrejeambrun @bbovenzi, I just pushed a new commit that corrects the majority of the problems you pointed out. Let me know if everything is as expected and if I didn't miss any of your requests (except the 2 not resolved in the conversation). -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2155490842
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,96 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useQueryClient } from "@tanstack/react-query";
+import { useEffect, useMemo, useState } from "react";
+import { useTranslation } from "react-i18next";
+import { FiStar } from "react-icons/fi";
+import { useLocation } from "react-router-dom";
+
+import { useDagServiceGetDags } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+
+const MAX_VISIBLE = 5;
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { t: translate } = useTranslation("dashboard");
+ const { data: favorites } = useDagServiceGetDags({ favorites: true });
Review Comment:
> The rest of the UI is using `useDagServiceGetDagsUi` to fetch dags. The UI
should probably only use this, and we should also update the `ui/get_dags`
endpoint to be able to do the same filtering on favorites. (therefore a common
`parameters.py` will be helpful)
Currently working on this.
I will submit in the next few days another commit with the necessary changes
(it will probably involve a rebase, since I don't have the
**useDagServiceGetDagsUi** implemented in my local version).
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2155473060
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +358,55 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Mark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(insert(DagFavorite).values(dag_id=dag_id, user_id=user_id))
+session.commit()
+
+return DAGResponse.model_validate(dag)
+
+
+@dags_router.post(
+"/{dag_id}/unfavorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
+)
+def unfavorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Unmark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(
+delete(DagFavorite).where(
+DagFavorite.dag_id == dag_id,
+DagFavorite.user_id == user_id,
+)
+)
+session.commit()
+
+return DAGResponse.model_validate(dag)
Review Comment:
> We return a dag response, but this does not hold any information about the
`favorite/non favorite` state of the Dag, not sure if it's relevant.
With the current implementation, I don't think there is the need to return
specific information, but maybe I'm missing something.
What would you suggest? I was thinking that maybe we could just return the
200 successful code, if no additional information is needed to return. Similar
to what is done in the delete action, where the 204 code is returned.
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2155459393
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -113,9 +115,19 @@ def get_dags(
],
readable_dags_filter: ReadableDagsFilterDep,
session: SessionDep,
+user: GetUserDep,
+favorites: Annotated[bool | None, Query()] = None,
Review Comment:
Done.
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +358,55 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Mark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(insert(DagFavorite).values(dag_id=dag_id, user_id=user_id))
+session.commit()
+
+return DAGResponse.model_validate(dag)
+
+
+@dags_router.post(
+"/{dag_id}/unfavorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
+)
+def unfavorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Unmark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(
+delete(DagFavorite).where(
+DagFavorite.dag_id == dag_id,
+DagFavorite.user_id == user_id,
+)
+)
+session.commit()
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264: URL: https://github.com/apache/airflow/pull/51264#discussion_r2155457642 ## airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dags.py: ## @@ -392,6 +420,74 @@ def test_patch_dags_should_response_403(self, unauthorized_test_client): assert response.status_code == 403 +class TestFavoriteDag(TestDagEndpoint): +"""Unit tests for favoriting a DAG.""" + [email protected]( +"dag_id, expected_status_code, expected_exist_in_favorites", +[ +("fake_dag_id", 404, None), +(DAG1_ID, 200, True), +], +) +def test_favorite_dag( +self, test_client, dag_id, expected_status_code, expected_exist_in_favorites, session +): +response = test_client.post(f"/dags/{dag_id}/favorite") +assert response.status_code == expected_status_code + +if expected_status_code == 200: +result = session.execute( +select(DagFavorite).where(DagFavorite.dag_id == dag_id, DagFavorite.user_id == "test") +).first() +assert result is not None if expected_exist_in_favorites else result is None +check_last_log(session, dag_id=dag_id, event="favorite_dag", logical_date=None) + +def test_favorite_dag_should_response_401(self, unauthenticated_test_client): +response = unauthenticated_test_client.post(f"/dags/{DAG1_ID}/favorite") +assert response.status_code == 401 + +def test_favorite_dag_should_response_403(self, unauthorized_test_client): +response = unauthorized_test_client.post(f"/dags/{DAG1_ID}/favorite") +assert response.status_code == 403 + 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264: URL: https://github.com/apache/airflow/pull/51264#discussion_r2155457642 ## airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dags.py: ## @@ -392,6 +420,74 @@ def test_patch_dags_should_response_403(self, unauthorized_test_client): assert response.status_code == 403 +class TestFavoriteDag(TestDagEndpoint): +"""Unit tests for favoriting a DAG.""" + [email protected]( +"dag_id, expected_status_code, expected_exist_in_favorites", +[ +("fake_dag_id", 404, None), +(DAG1_ID, 200, True), +], +) +def test_favorite_dag( +self, test_client, dag_id, expected_status_code, expected_exist_in_favorites, session +): +response = test_client.post(f"/dags/{dag_id}/favorite") +assert response.status_code == expected_status_code + +if expected_status_code == 200: +result = session.execute( +select(DagFavorite).where(DagFavorite.dag_id == dag_id, DagFavorite.user_id == "test") +).first() +assert result is not None if expected_exist_in_favorites else result is None +check_last_log(session, dag_id=dag_id, event="favorite_dag", logical_date=None) + +def test_favorite_dag_should_response_401(self, unauthenticated_test_client): +response = unauthenticated_test_client.post(f"/dags/{DAG1_ID}/favorite") +assert response.status_code == 401 + +def test_favorite_dag_should_response_403(self, unauthorized_test_client): +response = unauthorized_test_client.post(f"/dags/{DAG1_ID}/favorite") +assert response.status_code == 403 + Review Comment: I just created the tests in my last commit. Let me know if everything is ok. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2155455005
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +358,55 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Mark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(insert(DagFavorite).values(dag_id=dag_id, user_id=user_id))
+session.commit()
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2155455274
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +358,55 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264: URL: https://github.com/apache/airflow/pull/51264#discussion_r2155455951 ## airflow-core/src/airflow/api_fastapi/core_api/datamodels/dags.py: ## @@ -121,6 +121,12 @@ class DAGPatchBody(StrictBaseModel): is_paused: bool +class DAGFavoriteBody(StrictBaseModel): +"""Dag Serializer for updatable bodies.""" 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2155453043
##
airflow-core/src/airflow/ui/src/components/DagActions/FavoriteDagButton.tsx:
##
@@ -0,0 +1,72 @@
+/*!
+ * 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 { Box } from "@chakra-ui/react";
+import { useCallback, useMemo } from "react";
+import { FiStar } from "react-icons/fi";
+
+import { useDagServiceGetFavoriteDags } from "openapi/queries";
+import { useFavoriteDag } from "src/queries/useFavoriteDag";
+
+import ActionButton from "../ui/ActionButton";
+
+type FavoriteDagButtonProps = {
+ readonly dagId: string;
+ readonly withText?: boolean;
+};
+
+export const FavoriteDagButton = ({ dagId, withText = true }:
FavoriteDagButtonProps) => {
+ const { data: favorites, refetch } = useDagServiceGetFavoriteDags();
+
+ const isFavorite = useMemo(
+() => favorites?.dags.some((fav) => fav.dag_id === dagId) ?? false,
+[favorites, dagId],
+ );
+
+ const { mutate: toggleFavorite } = useFavoriteDag({ dagId });
+
+ const onToggle = useCallback(() => {
+toggleFavorite(
+ {
+dagId,
+requestBody: {
+ is_favorite: !isFavorite,
+},
+ },
+ {
+onSuccess: () => {
+ void refetch();
+},
+ },
+);
+ }, [dagId, isFavorite, toggleFavorite, refetch]);
+
+ return (
+
+ 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 { Box } from "@chakra-ui/react";
+import { useCallback, useMemo } from "react";
+import { FiStar } from "react-icons/fi";
+
+import { useDagServiceGetFavoriteDags } from "openapi/queries";
+import { useFavoriteDag } from "src/queries/useFavoriteDag";
+
+import ActionButton from "../ui/ActionButton";
+
+type FavoriteDagButtonProps = {
+ readonly dagId: string;
+ readonly withText?: boolean;
+};
+
+export const FavoriteDagButton = ({ dagId, withText = true }:
FavoriteDagButtonProps) => {
+ const { data: favorites, refetch } = useDagServiceGetFavoriteDags();
+
+ const isFavorite = useMemo(
+() => favorites?.dags.some((fav) => fav.dag_id === dagId) ?? false,
+[favorites, dagId],
+ );
+
+ const { mutate: toggleFavorite } = useFavoriteDag({ dagId });
+
+ const onToggle = useCallback(() => {
+toggleFavorite(
+ {
+dagId,
+requestBody: {
+ is_favorite: !isFavorite,
+},
+ },
+ {
+onSuccess: () => {
+ void refetch();
+},
+ },
+);
+ }, [dagId, isFavorite, toggleFavorite, refetch]);
+
+ return (
+
+
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2155453043
##
airflow-core/src/airflow/ui/src/components/DagActions/FavoriteDagButton.tsx:
##
@@ -0,0 +1,72 @@
+/*!
+ * 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 { Box } from "@chakra-ui/react";
+import { useCallback, useMemo } from "react";
+import { FiStar } from "react-icons/fi";
+
+import { useDagServiceGetFavoriteDags } from "openapi/queries";
+import { useFavoriteDag } from "src/queries/useFavoriteDag";
+
+import ActionButton from "../ui/ActionButton";
+
+type FavoriteDagButtonProps = {
+ readonly dagId: string;
+ readonly withText?: boolean;
+};
+
+export const FavoriteDagButton = ({ dagId, withText = true }:
FavoriteDagButtonProps) => {
+ const { data: favorites, refetch } = useDagServiceGetFavoriteDags();
+
+ const isFavorite = useMemo(
+() => favorites?.dags.some((fav) => fav.dag_id === dagId) ?? false,
+[favorites, dagId],
+ );
+
+ const { mutate: toggleFavorite } = useFavoriteDag({ dagId });
+
+ const onToggle = useCallback(() => {
+toggleFavorite(
+ {
+dagId,
+requestBody: {
+ is_favorite: !isFavorite,
+},
+ },
+ {
+onSuccess: () => {
+ void refetch();
+},
+ },
+);
+ }, [dagId, isFavorite, toggleFavorite, refetch]);
+
+ return (
+
+
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2155451344
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDagCard.tsx:
##
@@ -0,0 +1,67 @@
+/*!
+ * 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 { Box, Button, Text, VStack } from "@chakra-ui/react";
+import { Link as RouterLink } from "react-router-dom";
+
+import { useDagServiceRecentDagRuns } from "openapi/queries";
+import { RecentRuns } from "src/pages/DagsList/RecentRuns";
+
+type FavoriteDagProps = {
+ readonly dagId: string;
+};
+
+export const FavoriteDagCard = ({ dagId }: FavoriteDagProps) => {
+ const { data } = useDagServiceRecentDagRuns({
+dagIds: [dagId],
+dagRunsLimit: 5,
+ });
+
+ const latestRuns = data?.dags[0]?.latest_dag_runs ?? [];
+
+ return (
+
+
+
+
+
+
+
+{dagId}
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2137776555
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +358,55 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Mark the DAG as favorite."""
+dag = session.get(DagModel, dag_id)
+if not dag:
+raise HTTPException(status.HTTP_404_NOT_FOUND, detail=f"DAG with id
'{dag_id}' not found")
+
+user_id = user.get_id()
+session.execute(insert(DagFavorite).values(dag_id=dag_id, user_id=user_id))
+session.commit()
Review Comment:
No need to commit, SessionDep will do that for you when the request end.
##
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dags.py:
##
@@ -392,6 +420,74 @@ def test_patch_dags_should_response_403(self,
unauthorized_test_client):
assert response.status_code == 403
+class TestFavoriteDag(TestDagEndpoint):
+"""Unit tests for favoriting a DAG."""
+
[email protected](
+"dag_id, expected_status_code, expected_exist_in_favorites",
+[
+("fake_dag_id", 404, None),
+(DAG1_ID, 200, True),
+],
+)
+def test_favorite_dag(
+self, test_client, dag_id, expected_status_code,
expected_exist_in_favorites, session
+):
+response = test_client.post(f"/dags/{dag_id}/favorite")
+assert response.status_code == expected_status_code
+
+if expected_status_code == 200:
+result = session.execute(
+select(DagFavorite).where(DagFavorite.dag_id == dag_id,
DagFavorite.user_id == "test")
+).first()
+assert result is not None if expected_exist_in_favorites else
result is None
+check_last_log(session, dag_id=dag_id, event="favorite_dag",
logical_date=None)
+
+def test_favorite_dag_should_response_401(self,
unauthenticated_test_client):
+response =
unauthenticated_test_client.post(f"/dags/{DAG1_ID}/favorite")
+assert response.status_code == 401
+
+def test_favorite_dag_should_response_403(self, unauthorized_test_client):
+response = unauthorized_test_client.post(f"/dags/{DAG1_ID}/favorite")
+assert response.status_code == 403
+
Review Comment:
Can we get a test for favoriting a dag that is already a favorite ? This
should raise a 409 conflict error.
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +358,55 @@ def patch_dags(
)
+@dags_router.post(
+"/{dag_id}/favorite",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="POST")),
Depends(action_logging())],
Review Comment:
Maybe we can consider that any user that can `read a dag` can create its own
`favorite` of it? And therefore we only need the `GET` permission on the DAG to
be able to create a favorite of it?
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -113,9 +115,19 @@ def get_dags(
],
readable_dags_filter: ReadableDagsFilterDep,
session: SessionDep,
+user: GetUserDep,
+favorites: Annotated[bool | None, Query()] = None,
Review Comment:
```suggestion
is_favorite: Annotated[bool | None, Query()] = None,
```
I think we should create a reusable param out of this. Just to not clutter
the `route` file, and also because it will be reusable.
Basically create a parameter, you can take a look at `common/parameters.py`.
This will be more or less the `to_orm` method. Also I should be able to pass
`False` to retrieve non favorited dag only. (if a value is defined for the
filter, do the join and the filter, if it's None, don't do anything)
```python
.join(DagFavorite, DagModel.dag_id == DagFavorite.dag_id)
.where(DagFavorite.user_id == user_id)
```
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,96 @@
+/*!
+ * 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,
+ * soft
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
mariana-marcal-santana commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2134858608
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +368,39 @@ def patch_dags(
)
+@dags_router.put(
+"/{dag_id}",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="PUT")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+favorite_body: DAGFavoriteBody,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Favorite the specific DAG."""
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
mariana-marcal-santana commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2134858171
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -54,23 +54,45 @@
from airflow.api_fastapi.core_api.datamodels.dags import (
DAGCollectionResponse,
DAGDetailsResponse,
+DAGFavoriteBody,
DAGPatchBody,
DAGResponse,
)
from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.security import (
EditableDagsFilterDep,
+GetUserDep,
ReadableDagsFilterDep,
requires_access_dag,
)
from airflow.api_fastapi.logging.decorators import action_logging
from airflow.exceptions import AirflowException, DagNotFound
from airflow.models import DAG, DagModel
+from airflow.models.dag_favorite import DagFavorite
from airflow.models.dagrun import DagRun
dags_router = AirflowRouter(tags=["DAG"], prefix="/dags")
+@dags_router.get("/favorite",
dependencies=[Depends(requires_access_dag(method="GET"))])
+def get_favorite_dags(session: SessionDep, user: GetUserDep) ->
DAGCollectionResponse:
+"""Get DAGs favorited by the user."""
+user_id = user.get_id()
+
+favorite_dags_query = (
+select(DagModel)
+.join(DagFavorite, DagModel.dag_id == DagFavorite.dag_id)
+.where(DagFavorite.user_id == user_id)
+)
+
+dags = session.scalars(favorite_dags_query).all()
+
+return DAGCollectionResponse(
+dags=dags,
+total_entries=len(dags),
+)
Review Comment:
> I think we are missing filtering the dag_id by the `readable_dag_ids`
Since this endpoint has been merged in the GetDags endpoint, we believe this
has been fixed.
Thank you
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
mariana-marcal-santana commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2134857892
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -54,23 +54,45 @@
from airflow.api_fastapi.core_api.datamodels.dags import (
DAGCollectionResponse,
DAGDetailsResponse,
+DAGFavoriteBody,
DAGPatchBody,
DAGResponse,
)
from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.security import (
EditableDagsFilterDep,
+GetUserDep,
ReadableDagsFilterDep,
requires_access_dag,
)
from airflow.api_fastapi.logging.decorators import action_logging
from airflow.exceptions import AirflowException, DagNotFound
from airflow.models import DAG, DagModel
+from airflow.models.dag_favorite import DagFavorite
from airflow.models.dagrun import DagRun
dags_router = AirflowRouter(tags=["DAG"], prefix="/dags")
+@dags_router.get("/favorite",
dependencies=[Depends(requires_access_dag(method="GET"))])
Review Comment:
> not sure about `dags/favorite` maybe that should be on it's own root path
`/favorites`. (also always plural). Or even better, maybe just a filter option
in the `get_dags` endpoint (probably only the UI one). `favorite: true/false`.
So we can beneficiate from the full features of the get_dags endpoint but also
on favorites. (for instance GET all the favorite dags with a dag_id_pattern
that is ordering_by something else)
>
> Maybe that sould be actions on the dag. `dags/{dag_id}/favorite`
`dags/{dag_id}/unfavorite`. (POST on both) So we don't even need a body. To
mark a dag as favorite or unfavorite.
@pierrejeambrun
The suggested changes are in our latest commit.
Could you give us some feedback if possible?
Thank you
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2130674757
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,94 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useEffect, useMemo, useState } from "react";
+import { FiStar } from "react-icons/fi";
+
+import { useDagServiceGetFavoriteDags } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+import { useQueryClient } from "@tanstack/react-query";
+import { useLocation } from "react-router-dom";
+
+const MAX_VISIBLE = 5;
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { data: favorites } = useDagServiceGetFavoriteDags();
+
+ useEffect(() => {
+queryClient.refetchQueries({ queryKey: ['DagServiceGetFavoriteDags'] });
+ }, [location.key, queryClient]);
+
+ const [showAll, setShowAll] = useState(false);
+
+ const visibleFavorites = useMemo(() => {
+if (!favorites?.dags) {
+ return [];
+}
+
+return showAll ? favorites.dags : favorites.dags.slice(0, MAX_VISIBLE);
+ }, [favorites, showAll]);
+
+ if (!favorites) {
+return null;
+ }
+
+ return (
+
+
+
+
+ Favorite Dags
+
+
+
+ {favorites.dags.length === 0 ? (
+
+ No favorites yet. Click the star icon next to a DAG in the list to
add it to your favorites.
+
+ ) : (
+<>
+
+{visibleFavorites.map((dag) => (
+
+))}
+
+
+ {favorites.total_entries > MAX_VISIBLE && (
+
+
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on PR #51264: URL: https://github.com/apache/airflow/pull/51264#issuecomment-2946565885 > Looks good in general in my view. Besides th i18n also DB migration is missing allowing to upgrade existing setups. > Thanks for the PR, LGMT. > > FYI: https://github.com/apache/airflow/blob/main/contributing-docs/14_metadata_database_updates.rst for fixing database migrations The DB migration should be present now. Let me know if anything is still missing or incomplete. -- 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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
PNL0 commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2130623257
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,94 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useEffect, useMemo, useState } from "react";
+import { FiStar } from "react-icons/fi";
+
+import { useDagServiceGetFavoriteDags } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+import { useQueryClient } from "@tanstack/react-query";
+import { useLocation } from "react-router-dom";
+
+const MAX_VISIBLE = 5;
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { data: favorites } = useDagServiceGetFavoriteDags();
+
+ useEffect(() => {
+queryClient.refetchQueries({ queryKey: ['DagServiceGetFavoriteDags'] });
+ }, [location.key, queryClient]);
+
+ const [showAll, setShowAll] = useState(false);
+
+ const visibleFavorites = useMemo(() => {
+if (!favorites?.dags) {
+ return [];
+}
+
+return showAll ? favorites.dags : favorites.dags.slice(0, MAX_VISIBLE);
+ }, [favorites, showAll]);
+
+ if (!favorites) {
+return null;
+ }
+
+ return (
+
+
+
+
+ Favorite Dags
+
+
+
+ {favorites.dags.length === 0 ? (
+
+ No favorites yet. Click the star icon next to a DAG in the list to
add it to your favorites.
Review Comment:
I just made a new commit to use the i18n module.
Let me know if anything is still missing.
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bbovenzi commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2127098336
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -54,23 +54,45 @@
from airflow.api_fastapi.core_api.datamodels.dags import (
DAGCollectionResponse,
DAGDetailsResponse,
+DAGFavoriteBody,
DAGPatchBody,
DAGResponse,
)
from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.security import (
EditableDagsFilterDep,
+GetUserDep,
ReadableDagsFilterDep,
requires_access_dag,
)
from airflow.api_fastapi.logging.decorators import action_logging
from airflow.exceptions import AirflowException, DagNotFound
from airflow.models import DAG, DagModel
+from airflow.models.dag_favorite import DagFavorite
from airflow.models.dagrun import DagRun
dags_router = AirflowRouter(tags=["DAG"], prefix="/dags")
+@dags_router.get("/favorite",
dependencies=[Depends(requires_access_dag(method="GET"))])
Review Comment:
+1 on filtering the existing dags list by `favorite: bool`
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bbovenzi commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2127103050
##
airflow-core/src/airflow/ui/src/components/DagActions/FavoriteDagButton.tsx:
##
@@ -0,0 +1,72 @@
+/*!
+ * 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 { Box } from "@chakra-ui/react";
+import { useCallback, useMemo } from "react";
+import { FiStar } from "react-icons/fi";
+
+import { useDagServiceGetFavoriteDags } from "openapi/queries";
+import { useFavoriteDag } from "src/queries/useFavoriteDag";
+
+import ActionButton from "../ui/ActionButton";
+
+type FavoriteDagButtonProps = {
+ readonly dagId: string;
+ readonly withText?: boolean;
+};
+
+export const FavoriteDagButton = ({ dagId, withText = true }:
FavoriteDagButtonProps) => {
+ const { data: favorites, refetch } = useDagServiceGetFavoriteDags();
+
+ const isFavorite = useMemo(
+() => favorites?.dags.some((fav) => fav.dag_id === dagId) ?? false,
+[favorites, dagId],
+ );
+
+ const { mutate: toggleFavorite } = useFavoriteDag({ dagId });
+
+ const onToggle = useCallback(() => {
+toggleFavorite(
+ {
+dagId,
+requestBody: {
+ is_favorite: !isFavorite,
+},
+ },
+ {
+onSuccess: () => {
+ void refetch();
+},
+ },
+);
+ }, [dagId, isFavorite, toggleFavorite, refetch]);
+
+ return (
+
+
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bbovenzi commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2127101071
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,94 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useEffect, useMemo, useState } from "react";
+import { FiStar } from "react-icons/fi";
+
+import { useDagServiceGetFavoriteDags } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+import { useQueryClient } from "@tanstack/react-query";
+import { useLocation } from "react-router-dom";
+
+const MAX_VISIBLE = 5;
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { data: favorites } = useDagServiceGetFavoriteDags();
+
+ useEffect(() => {
+queryClient.refetchQueries({ queryKey: ['DagServiceGetFavoriteDags'] });
+ }, [location.key, queryClient]);
+
+ const [showAll, setShowAll] = useState(false);
+
+ const visibleFavorites = useMemo(() => {
+if (!favorites?.dags) {
+ return [];
+}
+
+return showAll ? favorites.dags : favorites.dags.slice(0, MAX_VISIBLE);
+ }, [favorites, showAll]);
+
+ if (!favorites) {
+return null;
+ }
+
+ return (
+
+
+
+
+ Favorite Dags
+
+
+
+ {favorites.dags.length === 0 ? (
+
+ No favorites yet. Click the star icon next to a DAG in the list to
add it to your favorites.
+
+ ) : (
+<>
+
+{visibleFavorites.map((dag) => (
+
+))}
+
+
+ {favorites.total_entries > MAX_VISIBLE && (
+
+
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
bbovenzi commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2127100230
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDagCard.tsx:
##
@@ -0,0 +1,67 @@
+/*!
+ * 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 { Box, Button, Text, VStack } from "@chakra-ui/react";
+import { Link as RouterLink } from "react-router-dom";
+
+import { useDagServiceRecentDagRuns } from "openapi/queries";
+import { RecentRuns } from "src/pages/DagsList/RecentRuns";
+
+type FavoriteDagProps = {
+ readonly dagId: string;
+};
+
+export const FavoriteDagCard = ({ dagId }: FavoriteDagProps) => {
+ const { data } = useDagServiceRecentDagRuns({
+dagIds: [dagId],
+dagRunsLimit: 5,
+ });
+
+ const latestRuns = data?.dags[0]?.latest_dag_runs ?? [];
+
+ return (
+
+
+
+
+
+
+
+{dagId}
Review Comment:
Let's show the dag_display_name when possible
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2121508221
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -54,23 +54,45 @@
from airflow.api_fastapi.core_api.datamodels.dags import (
DAGCollectionResponse,
DAGDetailsResponse,
+DAGFavoriteBody,
DAGPatchBody,
DAGResponse,
)
from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.security import (
EditableDagsFilterDep,
+GetUserDep,
ReadableDagsFilterDep,
requires_access_dag,
)
from airflow.api_fastapi.logging.decorators import action_logging
from airflow.exceptions import AirflowException, DagNotFound
from airflow.models import DAG, DagModel
+from airflow.models.dag_favorite import DagFavorite
from airflow.models.dagrun import DagRun
dags_router = AirflowRouter(tags=["DAG"], prefix="/dags")
+@dags_router.get("/favorite",
dependencies=[Depends(requires_access_dag(method="GET"))])
Review Comment:
not sure about `dags/favorite` maybe that should be on it's own root path
`/favorites`. (also always plural). Or even better, maybe just a filter option
in the `get_dags` endpoint (probably only the UI one). `favorite: true/false`.
So we can beneficiate from the full features of the get_dags endpoint but also
on favorites. (for instance GET all the favorite dags with a dag_id_pattern
that is ordering_by something else)
Maybe that sould be actions on the dag. `dags/{dag_id}/favorite`
`dags/{dag_id}/unfavorite`. (POST on both) So we don't even need a body.
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2121508221
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -54,23 +54,45 @@
from airflow.api_fastapi.core_api.datamodels.dags import (
DAGCollectionResponse,
DAGDetailsResponse,
+DAGFavoriteBody,
DAGPatchBody,
DAGResponse,
)
from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.security import (
EditableDagsFilterDep,
+GetUserDep,
ReadableDagsFilterDep,
requires_access_dag,
)
from airflow.api_fastapi.logging.decorators import action_logging
from airflow.exceptions import AirflowException, DagNotFound
from airflow.models import DAG, DagModel
+from airflow.models.dag_favorite import DagFavorite
from airflow.models.dagrun import DagRun
dags_router = AirflowRouter(tags=["DAG"], prefix="/dags")
+@dags_router.get("/favorite",
dependencies=[Depends(requires_access_dag(method="GET"))])
Review Comment:
not sure about `dags/favorite` maybe that should be on it's own root path
`/favorites`. (also always plural). Or even better, maybe just a filter option
in the `get_dags` endpoint (probably only the UI one). `favorite: true/false`.
So we can beneficiate from the full features of the get_dags endpoint but also
on favorites. (for instance GET all the favorite dags with a dag_id_pattern
that is ordering_by something else)
Maybe that sould be actions on the dag. `dags/{dag_id}/favorite`
`dags/{dag_id}/unfavorite`. (POST on both) So we don't even need a body. To
mark a dag as favorite or unfavorite.
--
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]
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
pierrejeambrun commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2121508221
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -54,23 +54,45 @@
from airflow.api_fastapi.core_api.datamodels.dags import (
DAGCollectionResponse,
DAGDetailsResponse,
+DAGFavoriteBody,
DAGPatchBody,
DAGResponse,
)
from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.security import (
EditableDagsFilterDep,
+GetUserDep,
ReadableDagsFilterDep,
requires_access_dag,
)
from airflow.api_fastapi.logging.decorators import action_logging
from airflow.exceptions import AirflowException, DagNotFound
from airflow.models import DAG, DagModel
+from airflow.models.dag_favorite import DagFavorite
from airflow.models.dagrun import DagRun
dags_router = AirflowRouter(tags=["DAG"], prefix="/dags")
+@dags_router.get("/favorite",
dependencies=[Depends(requires_access_dag(method="GET"))])
Review Comment:
not sure about `dags/favorite` maybe that should be on it's own root patch
`favorites`. (also always plural). Or even better, maybe just a filter option
in the `get_dags` endpoint (probably only the UI one). `favorite: true/false`.
So we can beneficiate from the full features of the get_dags endpoint but also
on favorites. (for instance GET all the favorite dags with a dag_id_pattern
that is ordering_by something else)
Maybe that sould be actions on the dag. `dags/{dag_id}/favorite`
`dags/{dag_id}/unfavorite`. (POST on both) So we don't even need a body.
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -54,23 +54,45 @@
from airflow.api_fastapi.core_api.datamodels.dags import (
DAGCollectionResponse,
DAGDetailsResponse,
+DAGFavoriteBody,
DAGPatchBody,
DAGResponse,
)
from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.security import (
EditableDagsFilterDep,
+GetUserDep,
ReadableDagsFilterDep,
requires_access_dag,
)
from airflow.api_fastapi.logging.decorators import action_logging
from airflow.exceptions import AirflowException, DagNotFound
from airflow.models import DAG, DagModel
+from airflow.models.dag_favorite import DagFavorite
from airflow.models.dagrun import DagRun
dags_router = AirflowRouter(tags=["DAG"], prefix="/dags")
+@dags_router.get("/favorite",
dependencies=[Depends(requires_access_dag(method="GET"))])
+def get_favorite_dags(session: SessionDep, user: GetUserDep) ->
DAGCollectionResponse:
+"""Get DAGs favorited by the user."""
+user_id = user.get_id()
+
+favorite_dags_query = (
+select(DagModel)
+.join(DagFavorite, DagModel.dag_id == DagFavorite.dag_id)
+.where(DagFavorite.user_id == user_id)
+)
+
+dags = session.scalars(favorite_dags_query).all()
+
+return DAGCollectionResponse(
+dags=dags,
+total_entries=len(dags),
+)
Review Comment:
I think we are missing filtering the dag_id by the `readable_dag_ids`
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -54,23 +54,45 @@
from airflow.api_fastapi.core_api.datamodels.dags import (
DAGCollectionResponse,
DAGDetailsResponse,
+DAGFavoriteBody,
DAGPatchBody,
DAGResponse,
)
from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.security import (
EditableDagsFilterDep,
+GetUserDep,
ReadableDagsFilterDep,
requires_access_dag,
)
from airflow.api_fastapi.logging.decorators import action_logging
from airflow.exceptions import AirflowException, DagNotFound
from airflow.models import DAG, DagModel
+from airflow.models.dag_favorite import DagFavorite
from airflow.models.dagrun import DagRun
dags_router = AirflowRouter(tags=["DAG"], prefix="/dags")
+@dags_router.get("/favorite",
dependencies=[Depends(requires_access_dag(method="GET"))])
+def get_favorite_dags(session: SessionDep, user: GetUserDep) ->
DAGCollectionResponse:
+"""Get DAGs favorited by the user."""
Review Comment:
```suggestion
"""Get DAGs favorited by the user."""
```
Reword, `favorited` seems weird
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##
@@ -346,6 +368,39 @@ def patch_dags(
)
+@dags_router.put(
+"/{dag_id}",
+responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+dependencies=[Depends(requires_access_dag(method="PUT")),
Depends(action_logging())],
+)
+def favorite_dag(
+dag_id: str,
+favorite_body: DAGFavoriteBody,
+session: SessionDep,
+user: GetUserDep,
+) -> DAGResponse:
+"""Favorite the specific DAG."""
Review Comment:
`
Re: [PR] Add support for favorite/pin dags to dashboard [airflow]
jscheffl commented on code in PR #51264:
URL: https://github.com/apache/airflow/pull/51264#discussion_r2119160799
##
airflow-core/src/airflow/ui/src/pages/Dashboard/FavoriteDags/FavoriteDags.tsx:
##
@@ -0,0 +1,94 @@
+/*!
+ * 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 { Box, Flex, Heading, SimpleGrid, Text } from "@chakra-ui/react";
+import { useEffect, useMemo, useState } from "react";
+import { FiStar } from "react-icons/fi";
+
+import { useDagServiceGetFavoriteDags } from "openapi/queries";
+
+import { FavoriteDagCard } from "./FavoriteDagCard";
+import { useQueryClient } from "@tanstack/react-query";
+import { useLocation } from "react-router-dom";
+
+const MAX_VISIBLE = 5;
+
+export const FavoriteDags = () => {
+ const queryClient = useQueryClient();
+ const location = useLocation();
+
+ const { data: favorites } = useDagServiceGetFavoriteDags();
+
+ useEffect(() => {
+queryClient.refetchQueries({ queryKey: ['DagServiceGetFavoriteDags'] });
+ }, [location.key, queryClient]);
+
+ const [showAll, setShowAll] = useState(false);
+
+ const visibleFavorites = useMemo(() => {
+if (!favorites?.dags) {
+ return [];
+}
+
+return showAll ? favorites.dags : favorites.dags.slice(0, MAX_VISIBLE);
+ }, [favorites, showAll]);
+
+ if (!favorites) {
+return null;
+ }
+
+ return (
+
+
+
+
+ Favorite Dags
+
+
+
+ {favorites.dags.length === 0 ? (
+
+ No favorites yet. Click the star icon next to a DAG in the list to
add it to your favorites.
Review Comment:
As we are in the process of translating the UI to other languages - can you
please use the i18n module and add the new texts to translated properties? See
https://github.com/apache/airflow/issues/50861
--
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]
