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.

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

Connectors: Handle arbitrary names #5015

Merged
merged 6 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion web-common/src/features/connectors/ConnectorExplorer.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@
}

.message {
@apply pl-2 pr-3.5 pt-2 pb-2 text-gray-500;
@apply pl-2 pr-3.5 py-2;
@apply text-gray-500;
@apply text-wrap;
}
</style>
16 changes: 14 additions & 2 deletions web-common/src/features/connectors/olap/DatabaseSchemaEntry.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,18 @@
</button>

{#if showTables}
{#if typedData && typedData.length > 0}
{#if connector?.errorMessage}
<div class="message">{connector.errorMessage}</div>
{:else if !connector.driver || !connector.driver.name}
<div class="message">Connector not found</div>
{:else if !typedData || typedData.length === 0}
<div class="message">No tables found</div>
{:else if typedData.length > 0}
<ol>
{#each typedData as tableInfo (tableInfo)}
<TableEntry
connectorInstanceId={instanceId}
{instanceId}
driver={connector.driver.name}
connector={connectorName}
{database}
{databaseSchema}
Expand Down Expand Up @@ -91,4 +98,9 @@
.database-schema-entry:not(.open) .database-schema-entry-header {
@apply bg-white;
}

.message {
@apply pl-2 pr-3.5 py-2;
@apply text-gray-500;
}
</style>
28 changes: 21 additions & 7 deletions web-common/src/features/connectors/olap/TableEntry.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
makeTablePreviewHref,
} from "./olap-config";

export let connectorInstanceId: string;
export let instanceId: string;
export let driver: string;
export let connector: string;
export let database: string; // The backend interprets an empty string as the default database
export let databaseSchema: string; // The backend interprets an empty string as the default schema
Expand All @@ -25,16 +26,23 @@
let showSchema = false;

$: fullyQualifiedTableName = makeFullyQualifiedTableName(
driver,
database,
databaseSchema,
table,
);
$: tableId = `${connector}-${fullyQualifiedTableName}`;
$: href = makeTablePreviewHref(
driver,
connector,
database,
databaseSchema,
table,
);
$: href = makeTablePreviewHref(connector, database, databaseSchema, table);
$: open = $page.url.pathname === href;
</script>

<li aria-label={fullyQualifiedTableName} class="table-entry group" class:open>
<li aria-label={tableId} class="table-entry group" class:open>
<div class="table-entry-header {database ? 'pl-[58px]' : 'pl-[40px]'}">
<TableIcon size="14px" className="shrink-0 text-gray-400" />
<Tooltip alignment="start" location="right" distance={32}>
Expand All @@ -52,7 +60,7 @@
</Tooltip>
{#if hasUnsupportedDataTypes}
<UnsupportedTypesIndicator
instanceId={connectorInstanceId}
{instanceId}
{connector}
{database}
{databaseSchema}
Expand All @@ -62,9 +70,9 @@
<DropdownMenu.Root bind:open={contextMenuOpen}>
<DropdownMenu.Trigger asChild let:builder>
<ContextButton
id="more-actions-{fullyQualifiedTableName}"
id="more-actions-{tableId}"
tooltipText="More actions"
label="{fullyQualifiedTableName} actions menu trigger"
label="{tableId} actions menu trigger"
builders={[builder]}
suppressTooltip={contextMenuOpen}
>
Expand All @@ -77,7 +85,13 @@
side="right"
sideOffset={16}
>
<TableMenuItems {connector} {database} {databaseSchema} {table} />
<TableMenuItems
{driver}
{connector}
{database}
{databaseSchema}
{table}
/>
</DropdownMenu.Content>
</DropdownMenu.Root>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,21 @@

const { ai } = featureFlags;

export let driver: string;
export let connector: string;
export let database: string = "";
export let databaseSchema: string = "";
export let table: string;

$: isModelingSupportedForCurrentOlapDriver =
useIsModelingSupportedForCurrentOlapDriver($runtime.instanceId);
$: href = makeTablePreviewHref(connector, database, databaseSchema, table);
$: href = makeTablePreviewHref(
driver,
connector,
database,
databaseSchema,
table,
);
$: createDashboardFromTable = useCreateDashboardFromTableUIAction(
$runtime.instanceId,
connector,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import { runtime } from "../../../runtime-client/runtime-store";
import { featureFlags } from "../../feature-flags";
import { useCreateDashboardFromTableUIAction } from "../../metrics-views/ai-generation/generateMetricsView";
import { makeFullyQualifiedTableName } from "./olap-config";

export let connector: string;
export let database: string = "";
Expand All @@ -21,13 +20,6 @@

const { ai } = featureFlags;

$: fullyQualifiedTableName = makeFullyQualifiedTableName(
connector,
database,
databaseSchema,
table,
);

$: createDashboardFromTable = useCreateDashboardFromTableUIAction(
$runtime.instanceId,
connector,
Expand All @@ -48,7 +40,7 @@
<WorkspaceHeader
editable={false}
showInspectorToggle={false}
titleInput={fullyQualifiedTableName}
titleInput={table}
>
<svelte:fragment let:width={headerWidth} slot="cta">
{@const collapse = isHeaderWidthSmall(headerWidth)}
Expand Down
25 changes: 12 additions & 13 deletions web-common/src/features/connectors/olap/olap-config.ts
Original file line number Diff line number Diff line change
@@ -1,43 +1,42 @@
export const OLAP_DRIVERS_WITHOUT_MODELING = ["clickhouse", "druid", "pinot"];

export function makeFullyQualifiedTableName(
connector: string,
driver: string,
database: string,
databaseSchema: string,
table: string,
) {
switch (connector) {
switch (driver) {
case "clickhouse":
return `${databaseSchema}.${table}`;
case "druid":
return `${databaseSchema}.${table}`;
case "duckdb":
// return `${database}.${databaseSchema}.${table}`;
// For now, only show the table name
return table;
return `${database}.${databaseSchema}.${table}`;
case "pinot":
return table;
default:
throw new Error(`Unsupported OLAP connector: ${connector}`);
throw new Error(`Unsupported OLAP connector: ${driver}`);
}
}

export function makeTablePreviewHref(
connector: string,
driver: string,
connectorName: string,
database: string,
databaseSchema: string,
table: string,
): string {
switch (connector) {
switch (driver) {
case "clickhouse":
return `/connector/clickhouse/${databaseSchema}/${table}`;
return `/connector/clickhouse/${connectorName}/${databaseSchema}/${table}`;
case "druid":
return `/connector/druid/${databaseSchema}/${table}`;
return `/connector/druid/${connectorName}/${databaseSchema}/${table}`;
case "duckdb":
return `/connector/duckdb/${database}/${databaseSchema}/${table}`;
return `/connector/duckdb/${connectorName}/${database}/${databaseSchema}/${table}`;
case "pinot":
return `/connector/pinot/${table}`;
return `/connector/pinot/${connectorName}/${table}`;
default:
throw new Error(`Unsupported connector: ${connector}`);
throw new Error(`Unsupported connector: ${driver}`);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

const { readOnly } = featureFlags;

$: name = $page.params.name;
$: database = $page.params.database;
// ClickHouse does not have a database "schema" concept
// Rill considers the ClickHouse "database" as the "database schema"
Expand All @@ -23,8 +24,4 @@
<title>Rill Developer | {table}</title>
</svelte:head>

<TablePreviewWorkspace
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason to keep individual pages for different connectors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally did it because each database has its own specific hierarchy (see below), but actually we might be able to use a [...catchall] route. I'll try that after we get the release out.

image

connector="clickhouse"
databaseSchema={database}
{table}
/>
<TablePreviewWorkspace connector={name} databaseSchema={database} {table} />
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

const { readOnly } = featureFlags;

$: name = $page.params.name;
// Druid does not have a "database" concept
$: databaseSchema = $page.params.schema;
$: table = $page.params.table;
Expand All @@ -22,4 +23,4 @@
<title>Rill Developer | {table}</title>
</svelte:head>

<TablePreviewWorkspace connector="druid" {databaseSchema} {table} />
<TablePreviewWorkspace connector={name} {databaseSchema} {table} />
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

const { readOnly } = featureFlags;

$: name = $page.params.name;
$: database = $page.params.database;
$: databaseSchema = $page.params.schema;
$: table = $page.params.table;
Expand All @@ -22,4 +23,4 @@
<title>Rill Developer | {table}</title>
</svelte:head>

<TablePreviewWorkspace connector="duckdb" {database} {databaseSchema} {table} />
<TablePreviewWorkspace connector={name} {database} {databaseSchema} {table} />
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

const { readOnly } = featureFlags;

$: name = $page.params.name;
$: table = $page.params.table;

onMount(() => {
Expand All @@ -20,4 +21,4 @@
<title>Rill Developer | {table}</title>
</svelte:head>

<TablePreviewWorkspace connector="pinot" {table} />
<TablePreviewWorkspace connector={name} {table} />
Loading