Skip to content

fix(new-webui): Add support for querying metadata from multiple datasets (fixes #1024). #1042

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 31 commits into from
Jul 9, 2025
Merged

fix(new-webui): Add support for querying metadata from multiple datasets (fixes #1024). #1042

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
a4d1cd8
latest
Jun 26, 2025
dab039b
latest
Jun 26, 2025
89a8b04
latest
Jun 26, 2025
0786e2c
latest
Jun 26, 2025
9f33330
latest
Jun 26, 2025
6e90915
latest
Jun 26, 2025
038656f
latest
Jun 27, 2025
1f0dc43
latest
Jun 27, 2025
01a134f
latest
Jun 27, 2025
e1bdcd5
latest
Jun 27, 2025
0191277
latest
Jun 27, 2025
7a9846d
latest
Jun 27, 2025
f9512b9
latest
Jun 30, 2025
e463470
latest
Jun 30, 2025
4bdc128
Merge branch 'main' into datasetfix
Jul 7, 2025
36d91fb
latest
Jul 7, 2025
6b72a6d
revert accidental change
Jul 7, 2025
4235ed6
revert accidental change
Jul 7, 2025
e7824cd
latest
Jul 7, 2025
55a3e80
latest
Jul 7, 2025
279e7ad
Merge branch 'main' into datasetfix
davemarco Jul 7, 2025
77b15c9
Merge branch 'main' into datasetfix
davemarco Jul 8, 2025
21e8eb2
latest
Jul 8, 2025
481e230
Merge branch 'datasetfix' of https://github.com/davemarco/clp into da…
Jul 8, 2025
661262c
latest
Jul 8, 2025
883f849
latest
Jul 8, 2025
f014a50
latest
Jul 8, 2025
f62a620
latest
Jul 8, 2025
968cc80
latest
Jul 8, 2025
84d80be
latest
Jul 8, 2025
a80a7dd
Merge branch 'main' into datasetfix
Jul 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions components/webui/client/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions components/webui/client/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
"@emotion/styled": "^11.14.0",
"@mui/joy": "^5.0.0-beta.51",
"@sinclair/typebox": "^0.34.25",
"@tanstack/react-query": "^5.81.4",
"@tanstack/react-query-devtools": "^5.81.4",
"antd": "^5.24.5",
"axios": "^1.7.9",
"chart.js": "^4.4.9",
Expand Down
16 changes: 11 additions & 5 deletions components/webui/client/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import {RouterProvider} from "react-router";

import {QueryClientProvider} from "@tanstack/react-query";
import {ReactQueryDevtools} from "@tanstack/react-query-devtools";
import {ConfigProvider} from "antd";

import queryClient from "./config/queryClient";
import router from "./router";
import THEME_CONFIG from "./theme";

Expand All @@ -15,11 +18,14 @@ import "@ant-design/v5-patch-for-react-19";
*/
const App = () => {
return (
<ConfigProvider
theme={THEME_CONFIG}
>
<RouterProvider router={router}/>
</ConfigProvider>
<QueryClientProvider client={queryClient}>
<ConfigProvider
theme={THEME_CONFIG}
>
<RouterProvider router={router}/>
</ConfigProvider>
<ReactQueryDevtools initialIsOpen={false}/>
</QueryClientProvider>
);
};

Expand Down
14 changes: 14 additions & 0 deletions components/webui/client/src/api/sql.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import axios from "axios";


/**
* Query the SQL server with the queryString.
*
* @param queryString
* @return
*/
const querySql = async <T>(queryString: string) => {
return axios.post<T>("/query/sql", {queryString});
};

export {querySql};
Copy link
Contributor

@hoophalab hoophalab Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original idea of having separate sql.ts is: the developer of ingestion page don't need to touch any other code -- every change can be done in src/pages/IngestPage.

However, this idea doesn't work. Since you are already create an src/api folder, then let's do it properly:

  1. create a API module that one-to-one maps all restful endpoint to a typescript function.
  2. create a backend manager class on top of the thin api that includes all shared fetchers such as fetchDatasetNames.

Copy link
Contributor Author

@davemarco davemarco Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for 1. i think the src/api is already the api module no?.
for 2. I dont think we need a class since there is no saved state. But i think we can move shared fetchers like fetchDatasetNames to src/api/ maybe in a datasets.ts file

Copy link
Contributor

@hoophalab hoophalab Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for 1. i think the src/api is already the api module no?

yes, but everything is mixed now. Can we create a module one-to-one maps each endpoint that fastify exposes?

for 2. I dont think we need a class since there is no saved state. But i think we can move shared fetchers like fetchDatasetNames to src/api/ maybe in a datasets.ts file

yes, it doesn't need to be a class. but we should move shared fetchers to src/api/

Copy link
Contributor Author

@davemarco davemarco Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for 1. do you mean like a new folder for each. I think that is okay

Copy link
Contributor

@hoophalab hoophalab Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay I prefer one as lesser of two evils

If we go with that approach, then I feel it's best to:

  1. We keep src/api/search.ts, src/api/query.ts, and src/api/sql.ts as they are.

  2. We move the other fetchers into src/fetchers.ts.

Copy link
Contributor Author

@davemarco davemarco Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about src/api/fetchers.ts?

Copy link
Contributor Author

@davemarco davemarco Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the fetchers are making requests so okay to be in the api folder, and keep one file out of the top level code

Copy link
Contributor

@hoophalab hoophalab Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sure. Let's put them in src/api/fetchers.ts.

Also, I’m thinking about adding a generic fetchSqlFirstRow function. A sketch

const fetchSqlFirstRow = async <T>(queryString: string): Promise<T> => {
    const resp = await querySql<T[]>(queryString);
    const [firstRow] = resp.data;
    if ("undefined" === typeof firstRow) {
        throw new Error(`SQL ${queryString} does not contain data.`);
    }
    return firstRow;
};

This way, we can keep all the build*Sql functions inside src/pages/IngestPage which aligns with Junhao’s original idea.

Copy link
Contributor Author

@davemarco davemarco Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont think this works since some like the dataset fetch multiple rows.

11 changes: 10 additions & 1 deletion components/webui/client/src/components/DashboardCard/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface DashboardCardProps {
titleColor?: string;
backgroundColor?: string;
children?: React.ReactNode;
isLoading?: boolean;
}

/**
Expand All @@ -23,13 +24,21 @@ interface DashboardCardProps {
* @param props.titleColor
* @param props.backgroundColor
* @param props.children
* @param props.isLoading
* @return
*/
const DashboardCard = ({title, titleColor, backgroundColor, children}: DashboardCardProps) => {
const DashboardCard = ({
title,
titleColor,
backgroundColor,
children,
isLoading = false,
}: DashboardCardProps) => {
return (
<Card
className={styles["card"] || ""}
hoverable={true}
loading={isLoading}
style={{backgroundColor}}
>
<div className={styles["cardContent"]}>
Expand Down
4 changes: 4 additions & 0 deletions components/webui/client/src/components/StatCard/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ interface StatCardProps {
backgroundColor?: string;
statSize?: string;
statColor?: string;
isLoading?: boolean;
}

/**
Expand All @@ -27,6 +28,7 @@ interface StatCardProps {
* @param props.backgroundColor
* @param props.statSize
* @param props.statColor
* @param props.isLoading
* @return
*/
const StatCard = ({
Expand All @@ -36,9 +38,11 @@ const StatCard = ({
backgroundColor,
statSize,
statColor,
isLoading = false,
}: StatCardProps) => {
const props: DashboardCardProps = {
title,
isLoading,
...(titleColor ?
{titleColor} :
{}),
Expand Down
14 changes: 14 additions & 0 deletions components/webui/client/src/config/queryClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import {QueryClient} from "@tanstack/react-query";


const DEFAULT_STALE_TIME_MILLIS = 10_000;

const queryClient = new QueryClient({
defaultOptions: {
queries: {
staleTime: DEFAULT_STALE_TIME_MILLIS,
},
},
});

export default queryClient;
5 changes: 4 additions & 1 deletion components/webui/client/src/pages/IngestPage/Details/DetailsCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import StatCard from "../../../components/StatCard";
interface DetailsCardProps {
title: string;
stat: string;
isLoading: boolean;
}

/**
Expand All @@ -14,12 +15,14 @@ interface DetailsCardProps {
* @param props
* @param props.title
* @param props.stat
* @param props.isLoading
* @return
*/
const DetailsCard = ({title, stat}: DetailsCardProps) => {
const DetailsCard = ({title, stat, isLoading}: DetailsCardProps) => {
Copy link
Contributor

@hoophalab hoophalab Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This inheritance-style dependencies is too crazy.
We have
DetailsCard -> StatCard -> DashboardCard
You are passing isLoading all the way down to DashboardCard which is a typical problem of inheritance

Suggestion:

  1. Combine all three into one: DetailsCard as all cards requires title and stat/content
  2. DetailsCard accepts title as a property
  3. DetailsCard accepts content text (i.e. No timestamp data and 0.00%) as react node in children, so that space saving can have a different fontsize.
  4. two options:
    1. All other cards uses DetailsCard directly and isLoading becomes a page state in zustand store
    2. Have a ...props parameter in all concrete cards such as File Message... which are directed to DetailsCard

Copy link
Contributor Author

@davemarco davemarco Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did also think these cards can be refactored when working on the code. But probably a different PR.

Copy link
Contributor

@hoophalab hoophalab Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we create a quick refactor PR to address this before merging? It shouldn’t take long — I’m happy to submit one and you review that. However, we shouldn't let the current isLoading solution make it into the main branch.

Copy link
Contributor Author

@davemarco davemarco Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay this is reasonable, but you will have to merge into this PR

const {token} = theme.useToken();
return (
<StatCard
isLoading={isLoading}
stat={stat}
statColor={token.colorTextSecondary}
statSize={"1.4rem"}
Expand Down
5 changes: 4 additions & 1 deletion components/webui/client/src/pages/IngestPage/Details/Files.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,21 @@ import DetailsCard from "./DetailsCard";

interface FilesProps {
numFiles: Nullable<number>;
isLoading: boolean;
}

/**
* Renders the files statistic.
*
* @param props
* @param props.numFiles
* @param props.isLoading
* @return
*/
const Files = ({numFiles}: FilesProps) => {
const Files = ({numFiles, isLoading}: FilesProps) => {
return (
<DetailsCard
isLoading={isLoading}
stat={(numFiles ?? 0).toString()}
title={"Files"}/>
);
Expand Down
5 changes: 4 additions & 1 deletion components/webui/client/src/pages/IngestPage/Details/Messages.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,21 @@ import DetailsCard from "./DetailsCard";

interface MessagesProps {
numMessages: Nullable<number>;
isLoading: boolean;
}

/**
* Renders the messages statistic.
*
* @param props
* @param props.numMessages
* @param props.isLoading
* @return
*/
const Messages = ({numMessages}: MessagesProps) => {
const Messages = ({numMessages, isLoading}: MessagesProps) => {
return (
<DetailsCard
isLoading={isLoading}
stat={(numMessages ?? 0).toString()}
title={"Messages"}/>
);
Expand Down
5 changes: 4 additions & 1 deletion components/webui/client/src/pages/IngestPage/Details/TimeRange.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const DATE_FORMAT = "MMMM D, YYYY";
interface TimeRangeProps {
beginDate: Dayjs;
endDate: Dayjs;
isLoading: boolean;
}

/**
Expand All @@ -16,9 +17,10 @@ interface TimeRangeProps {
* @param props
* @param props.beginDate
* @param props.endDate
* @param props.isLoading
* @return
*/
const TimeRange = ({beginDate, endDate}: TimeRangeProps) => {
const TimeRange = ({beginDate, endDate, isLoading}: TimeRangeProps) => {
let stat;
if (beginDate.isValid() && endDate.isValid()) {
stat = `${beginDate.format(DATE_FORMAT)} - ${endDate.format(DATE_FORMAT)}`;
Expand All @@ -28,6 +30,7 @@ const TimeRange = ({beginDate, endDate}: TimeRangeProps) => {

return (
<DetailsCard
isLoading={isLoading}
stat={stat}
title={"Time Range"}/>
);
Expand Down
Loading