Re: [PR] test(hot-path): upstream -> service -> route [apisix-dashboard]
SkyeYoung merged PR #3076: URL: https://github.com/apache/apisix-dashboard/pull/3076 -- 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: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] test(hot-path): upstream -> service -> route [apisix-dashboard]
SkyeYoung commented on code in PR #3076:
URL: https://github.com/apache/apisix-dashboard/pull/3076#discussion_r2103665574
##
src/apis/services.ts:
##
@@ -50,3 +50,16 @@ export const postServiceReq = (req: AxiosInstance, data:
ServicePostType) =>
API_SERVICES,
data
);
+
+export const deleteAllServices = async (req: AxiosInstance) => {
+ while (true) {
Review Comment:
done
##
src/apis/routes.ts:
##
@@ -43,12 +43,14 @@ export const postRouteReq = (req: AxiosInstance, data:
RoutePostType) =>
req.post(API_ROUTES, data);
export const deleteAllRoutes = async (req: AxiosInstance) => {
- const res = await getRouteListReq(req, {
-page: 1,
-page_size: 1000,
- });
- if (res.total === 0) return;
- return await Promise.all(
-res.list.map((d) => req.delete(`${API_ROUTES}/${d.value.id}`))
- );
+ while (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: notifications-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] test(hot-path): upstream -> service -> route [apisix-dashboard]
LiteSun commented on code in PR #3076:
URL: https://github.com/apache/apisix-dashboard/pull/3076#discussion_r2103635342
##
src/apis/routes.ts:
##
@@ -43,12 +43,14 @@ export const postRouteReq = (req: AxiosInstance, data:
RoutePostType) =>
req.post(API_ROUTES, data);
export const deleteAllRoutes = async (req: AxiosInstance) => {
- const res = await getRouteListReq(req, {
-page: 1,
-page_size: 1000,
- });
- if (res.total === 0) return;
- return await Promise.all(
-res.list.map((d) => req.delete(`${API_ROUTES}/${d.value.id}`))
- );
+ while (true) {
Review Comment:
It’s better to retrieve the total count in the first request, and then
perform the deletion based on that total number.
##
src/apis/services.ts:
##
@@ -50,3 +50,16 @@ export const postServiceReq = (req: AxiosInstance, data:
ServicePostType) =>
API_SERVICES,
data
);
+
+export const deleteAllServices = async (req: AxiosInstance) => {
+ while (true) {
Review Comment:
ditto
--
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: notifications-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] test(hot-path): upstream -> service -> route [apisix-dashboard]
Copilot commented on code in PR #3076:
URL: https://github.com/apache/apisix-dashboard/pull/3076#discussion_r2099869911
##
src/routes/routes/add.tsx:
##
@@ -39,12 +39,15 @@ const RouteAddForm = () => {
const postRoute = useMutation({
mutationFn: (d: RoutePostType) => postRouteReq(req, pipeProduce()(d)),
-async onSuccess() {
+async onSuccess(res) {
Review Comment:
[nitpick] Verify that the response object includes a valid data.value.id for
navigation to the route detail page; a brief inline comment might help future
maintainers.
##
src/apis/services.ts:
##
@@ -50,3 +50,16 @@ export const postServiceReq = (req: AxiosInstance, data:
ServicePostType) =>
API_SERVICES,
data
);
+
+export const deleteAllServices = async (req: AxiosInstance) => {
Review Comment:
The deleteAllServices function employs a while(true) loop to remove services
in batches; consider introducing a break condition or iteration cap to avoid
unintended endless loops.
```suggestion
export const deleteAllServices = async (req: AxiosInstance) => {
const MAX_ITERATIONS = 100; // Define a maximum iteration cap
let iterationCount = 0; // Initialize iteration counter
```
##
src/routes/services/add.tsx:
##
@@ -36,12 +36,15 @@ const ServiceAddForm = () => {
const postService = useMutation({
mutationFn: (d: ServicePostType) => postServiceReq(req, pipeProduce()(d)),
-async onSuccess() {
+async onSuccess(res) {
notifications.show({
message: t('info.add.success', { name: t('services.singular') }),
color: 'green',
});
- await router.navigate({ to: '/services' });
+ await router.navigate({
+to: '/services/detail/$id',
+params: { id: res.data.value.id },
+ });
Review Comment:
[nitpick] Ensure that res.data.value.id exists and is valid; consider adding
a comment or assertion if the response structure may change in the future.
##
src/apis/routes.ts:
##
@@ -43,12 +43,14 @@ export const postRouteReq = (req: AxiosInstance, data:
RoutePostType) =>
req.post(API_ROUTES, data);
export const deleteAllRoutes = async (req: AxiosInstance) => {
- const res = await getRouteListReq(req, {
-page: 1,
-page_size: 1000,
- });
- if (res.total === 0) return;
- return await Promise.all(
-res.list.map((d) => req.delete(`${API_ROUTES}/${d.value.id}`))
- );
+ while (true) {
Review Comment:
The use of a while(true) loop for deleting routes may lead to an infinite
loop if the deletion process does not reduce the total count; adding a
safeguard like a maximum iteration counter is recommended.
```suggestion
export const deleteAllRoutes = async (req: AxiosInstance) => {
const MAX_ITERATIONS = 100; // Safeguard to prevent infinite loops
let iterationCount = 0;
while (true) {
if (iterationCount >= MAX_ITERATIONS) {
throw new Error('Maximum iteration limit reached while deleting
routes.');
}
```
##
src/apis/upstreams.ts:
##
@@ -52,12 +52,14 @@ export const putUpstreamReq = (
};
export const deleteAllUpstreams = async (req: AxiosInstance) => {
- const res = await getUpstreamListReq(req, {
-page: 1,
-page_size: 1000,
- });
- if (res.total === 0) return;
- return await Promise.all(
-res.list.map((d) => req.delete(`${API_UPSTREAMS}/${d.value.id}`))
- );
+ while (true) {
Review Comment:
Using a while(true) loop for resource deletion can risk an infinite loop if
the API response remains unchanged; consider adding a maximum iteration limit
or error handling to prevent potential hangs.
```suggestion
const MAX_ITERATIONS = 100; // Maximum number of iterations to prevent
infinite loops
let iterationCount = 0;
while (iterationCount < MAX_ITERATIONS) {
```
--
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: notifications-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] test(hot-path): upstream -> service -> route [apisix-dashboard]
SkyeYoung commented on code in PR #3076:
URL: https://github.com/apache/apisix-dashboard/pull/3076#discussion_r2099800852
##
src/routes/routes/add.tsx:
##
@@ -39,12 +39,15 @@ const RouteAddForm = () => {
const postRoute = useMutation({
mutationFn: (d: RoutePostType) => postRouteReq(req, pipeProduce()(d)),
-async onSuccess() {
+async onSuccess(res) {
notifications.show({
message: t('info.add.success', { name: t('routes.singular') }),
color: 'green',
});
- await router.navigate({ to: '/routes' });
+ await router.navigate({
+to: '/routes/detail/$id',
+params: { id: res.data.value.id },
+ });
Review Comment:
It was a mistake to jump to the list page earlier. I think it should jump to
the detail page.
--
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: notifications-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
