Skip to content
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

Jump to bottom

feat(admin): Send SQL queries from the UI to system tables #2311

Merged
merged 2 commits into from
Dec 20, 2021
Merged

feat(admin): Send SQL queries from the UI to system tables #2311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4eaa4c3
feat(admin): Send SQL queries from the UI to system tables
lynnagara Dec 18, 2021
85b7dca
Merge branch 'master' into support-sql-queries
lynnagara Dec 20, 2021
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
2 changes: 1 addition & 1 deletion snuba/admin/dist/bundle.js
View file
Open in desktop

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions snuba/admin/static/api_client.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
} from "./runtime_config/types";

import {
ClickhouseCannedQuery,
ClickhouseNodeData,
QueryRequest,
QueryResult,
Expand All @@ -19,7 +18,6 @@ interface Client {
editConfig: (key: ConfigKey, value: ConfigValue) => Promise<Config>;
getAuditlog: () => Promise<ConfigChange[]>;
getClickhouseNodes: () => Promise<[ClickhouseNodeData]>;
getClickhouseCannedQueries: () => Promise<[ClickhouseCannedQuery]>;
executeQuery: (req: QueryRequest) => Promise<QueryResult>;
}

Expand Down Expand Up @@ -94,17 +92,19 @@ function Client() {
})
);
},
getClickhouseCannedQueries: () => {
const url = baseUrl + "clickhouse_queries";
return fetch(url).then((resp) => resp.json());
},
executeQuery: (query: QueryRequest) => {
const url = baseUrl + "run_clickhouse_system_query";
return fetch(url, {
headers: { "Content-Type": "application/json" },
method: "POST",
body: JSON.stringify(query),
}).then((resp) => resp.json());
}).then((resp) => {
if (resp.ok) {
return resp.json();
} else {
return resp.json().then(Promise.reject.bind(Promise));
}
});
Comment on lines +101 to +107
Copy link
Member Author

@lynnagara lynnagara Dec 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This looks terrible but it seems to work. Should probably all be rewritten as async/await

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to add some UI container for displaying errors in general, and then make HTTP request logic more generic so every API call writes to the same error div or whatever.

This is perfectly good for what we already have, though

Copy link
Member Author

Choose a reason for hiding this comment

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

This code is intentionally free of UI elements - all of the methods here should just return or reject a promise. We can definitely add some system to display errors in a consistent manner across all pages but I don't know that it should be here. Currently this just uses window.alert to display the error message in as I didn't want to deal with styling + managing the state for those yet.

},
};
}
Expand Down
166 changes: 98 additions & 68 deletions snuba/admin/static/clickhouse_queries/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,12 @@ import Client from "../api_client";
import { Table } from "../table";
import { COLORS } from "../theme";

import {
ClickhouseNodeData,
ClickhouseCannedQuery,
QueryRequest,
QueryResult,
} from "./types";
import { ClickhouseNodeData, QueryRequest, QueryResult } from "./types";

type QueryState = Partial<QueryRequest>;

function ClickhouseQueries(props: { api: Client }) {
const [nodeData, setNodeData] = useState<ClickhouseNodeData[]>([]);
const [cannedQueries, setCannedQueries] = useState<ClickhouseCannedQuery[]>(
[]
);
const [query, setQuery] = useState<QueryState>({});
const [queryResultHistory, setQueryResultHistory] = useState<QueryResult[]>(
[]
Expand All @@ -26,9 +18,6 @@ function ClickhouseQueries(props: { api: Client }) {
props.api.getClickhouseNodes().then((res) => {
setNodeData(res);
});
props.api.getClickhouseCannedQueries().then((res) => {
setCannedQueries(res);
});
}, []);

function selectStorage(storage: string) {
Expand All @@ -52,20 +41,25 @@ function ClickhouseQueries(props: { api: Client }) {
});
}

function selectCannedQuery(queryName: string) {
function updateQuerySql(sql: string) {
setQuery((prevQuery) => {
return {
...prevQuery,
query_name: queryName,
sql,
};
});
}

function executeQuery() {
props.api.executeQuery(query as QueryRequest).then((result) => {
result.input_query = `${query.query_name}(${query.storage},${query.host}:${query.port})`;
setQueryResultHistory((prevHistory) => [result, ...prevHistory]);
});
props.api
.executeQuery(query as QueryRequest)
.then((result) => {
result.input_query = `${query.sql} (${query.storage},${query.host}:${query.port})`;
setQueryResultHistory((prevHistory) => [result, ...prevHistory]);
})
.catch((err) => {
window.alert(err?.error || "An error occurred");
});
}

function copyText(text: string) {
Expand All @@ -76,59 +70,60 @@ function ClickhouseQueries(props: { api: Client }) {
<div>
<form>
<h2>Construct a query</h2>
<select
value={query.storage || ""}
onChange={(evt) => selectStorage(evt.target.value)}
>
<option disabled value="">
Select a storage
</option>
{nodeData.map((storage) => (
<option key={storage.storage_name} value={storage.storage_name}>
{storage.storage_name}
</option>
))}
</select>
{query.storage && (
<select
value={
query.host && query.port ? `${query.host}:${query.port}` : ""
}
onChange={(evt) => selectHost(evt.target.value)}
>
<option disabled value="">
Select a host
</option>
{nodeData
.find((el) => el.storage_name === query.storage)
?.local_nodes.map((node, nodeIndex) => (
<option
key={`${node.host}:${node.port}`}
value={`${node.host}:${node.port}`}
>
{node.host}:{node.port}
<div>
<TextArea value={query.sql || ""} onChange={updateQuerySql} />
</div>
<div style={executeActionsStyle}>
<div>
<select
value={query.storage || ""}
onChange={(evt) => selectStorage(evt.target.value)}
style={selectStyle}
>
<option disabled value="">
Select a storage
</option>
{nodeData.map((storage) => (
<option key={storage.storage_name} value={storage.storage_name}>
{storage.storage_name}
</option>
))}
</select>
)}
{query.storage && query.host && query.port && (
<select
value={query.query_name || ""}
onChange={(evt) => selectCannedQuery(evt.target.value)}
>
<option disabled value="">
Select a query
</option>
{cannedQueries.map((cannedQuery) => (
<option key={`${cannedQuery.name}`} value={`${cannedQuery.name}`}>
{cannedQuery.name}: {cannedQuery.description}
</select>
<select
disabled={!query.storage}
value={
query.host && query.port ? `${query.host}:${query.port}` : ""
}
onChange={(evt) => selectHost(evt.target.value)}
style={selectStyle}
>
<option disabled value="">
Select a host
</option>
))}
</select>
)}
{query.storage && query.host && query.port && query.query_name && (
<button onClick={(_) => executeQuery()}>Execute query</button>
)}
{nodeData
.find((el) => el.storage_name === query.storage)
?.local_nodes.map((node) => (
<option
key={`${node.host}:${node.port}`}
value={`${node.host}:${node.port}`}
>
{node.host}:{node.port}
</option>
))}
</select>
</div>
<div>
<button
onClick={(_) => executeQuery()}
style={executeButtonStyle}
disabled={
!query.storage || !query.host || !query.port || !query.sql
}
>
Execute query
</button>
</div>
</div>
</form>
<div>
<h2>Query results</h2>
Expand All @@ -143,6 +138,7 @@ function ClickhouseQueries(props: { api: Client }) {
</button>
</div>,
])}
columnWidths={[1, 5]}
/>
</div>
</div>
Expand All @@ -156,6 +152,40 @@ const jsonStyle = {
borderRadius: 4,
backgroundColor: COLORS.BG_LIGHT,
marginBottom: 10,
wordBreak: "break-all" as const,
};

const executeActionsStyle = {
display: "flex",
justifyContent: "space-between",
marginTop: 8,
};

const executeButtonStyle = {
height: 30,
border: 0,
padding: "4px 20px",
};

const selectStyle = {
marginRight: 8,
height: 30,
};

function TextArea(props: {
value: string;
onChange: (nextValue: string) => void;
}) {
const { value, onChange } = props;
return (
<textarea
spellCheck={false}
value={value}
onChange={(evt) => onChange(evt.target.value)}
style={{ width: "100%", height: 100 }}
placeholder={"Write your query here"}
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this placeholder could be a valid query? like the one in your screenshot

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm just going to merge this as-is. Let's build canned queries soon 🙂

/>
);
}

export default ClickhouseQueries;
10 changes: 2 additions & 8 deletions snuba/admin/static/clickhouse_queries/types.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,11 @@ type ClickhouseNodeData = {
local_nodes: ClickhouseNode[];
};

type ClickhouseCannedQuery = {
description: string | null;
name: string;
sql: string;
};

type QueryRequest = {
storage: string;
host: string;
port: number;
query_name: string;
sql: string;
};

type QueryResultColumnMetadata = [string];
Expand All @@ -32,4 +26,4 @@ type QueryResult = {
rows: [QueryResultRow];
};

export { ClickhouseNodeData, ClickhouseCannedQuery, QueryRequest, QueryResult };
export { ClickhouseNodeData, QueryRequest, QueryResult };