-
Notifications
You must be signed in to change notification settings - Fork 164
Connectors JSON Schema #8632
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
base: main
Are you sure you want to change the base?
Connectors JSON Schema #8632
Conversation
274c8cd to
45c3e9f
Compare
|
bug: nit: Are we not going to add HTTPS connector as two step in this PR? fine to do in in another, but we're already here so might as well. Seeing we removed clickhosue specific forms, can we also split clickhouse-cloud and clickhouse into separate .ts? |
royendo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good progress on migrating connectors to JSON Schema. The schema files are well-structured and the JSONSchemaFormRenderer is now generic. However, there are architectural concerns that should be addressed to fully realize the vision from PR #8467.
ClickHouse is not using the JSON Schema system
ClickhouseFormRenderer.svelte renders fields by iterating over clickhouseUiState?.filteredProperties, which originates from the backend's configProperties:
// ClickhouseFormRenderer.svelte:61
{#each clickhouseUiState?.filteredProperties ?? [] as property (property.key)}
...
{#if property.type === ConnectorDriverPropertyType.TYPE_STRING ...}This means ClickHouse has a JSON Schema (schemas/clickhouse.ts) that's only used for validation, not rendering. The form structure still comes from the backend.
The unique ClickHouse patterns—connector type selector, Parameters/DSN tabs, conditional SSL/port behavior—should be modeled as JSON Schema extensions so JSONSchemaFormRenderer can render them:
- The connector type selector is already a radio enum pattern (
x-display: "radio") - Parameters/DSN tabs could use
x-display: "tabs"orx-input-mode(per the migration path in #8467) - Conditional field behavior already works via
allOf/if/then
Once modeled in the schema, ClickhouseFormRenderer.svelte can be deleted and ClickHouse can flow through the same path as other connectors.
Frontend still consumes backend configProperties/sourceProperties
The design doc vision was for JSON Schema to be the single source of truth. Currently, backend properties are still consumed in multiple places:
-
FormRenderer.svelte- renders entirely fromConnectorDriverProperty[] -
AddDataFormManager.ts:164-191, 430-441- derives properties, DSN detection, initial values -
MultiStepConnectorFlow.svelte:59-60, 77- usessourceProperties/configPropertiesfor initialization -
sourceUtils.ts:229-242- iterates over properties for form data preparation
For connectors with JSON Schemas, the schema should drive everything: field structure, labels, placeholders, hints, and validation. Backend properties should only be a fallback during migration.
What "done" looks like
Before we consider the JSON Schema migration complete, we need:
-
Tabs support in
JSONSchemaFormRenderer- Model the Parameters/DSN pattern as a schema extension (e.g.,x-display: "tabs"withx-tab-group). This unblocks ClickHouse and other connectors with dual input modes. -
ClickHouse rendering through
JSONSchemaFormRenderer- DeleteClickhouseFormRenderer.svelteonce the schema can express all its UI patterns. -
Schema-driven connectors stop reading
configProperties/sourceProperties- Derive field metadata from the schema. This may require enriching schemas withx-placeholder,x-hint, etc. where they're currently missing.
Without these, we have two parallel rendering systems that must be maintained independently.
Developed in collaboration with Claude Code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- When testing a connector, the creds are populated into the model SQL page.
- Going back doesnt remove bucket
- style added in copy paste
- connector info added to model
- Connection_mode added to YAML preview (not actual file) (snowflake)
connection_mode: dsn
-
create_secrets_from_connectors: httpsshowing even though two step not enabled -
If possible, move CHCloud to its own button
-
Druid, and other OLAP should not have model step (for now)
Sorry, what do you mean by moving CHCloud to its own button? @royendo |
|
@lovincyrus , like MotherDuck and Duckdb are separate icons for the same framework duckdb, ClickHouse Cloud and Clickhouse should be their own buttons :) Should have the SVG in Figma this will also simplify the CH button UI, lots of radio icons :) |
|
QA:
but nothing surfaces to UI. (had to make cli to get the error) |
royendo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment
The right sidebar was never supposed to be there from the Figma mocks. |
Sorry, I don't understand this. |
Out of scope. Let's track this elsewhere. This branch is for the connectors JSON schema files. |
Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The frontend should enumerate connectors from JSON Schemas, not from the backend b81376d
The modal in AddDataModal.svelte fetches ListConnectorDrivers from the backend and filters by hardcoded lists (SOURCES, OLAP_ENGINES). But if JSON Schema is the single source of truth for form rendering, it should also be the source of truth for which connectors appear in the UI.
The JSON Schemas should include metadata tags (e.g., x-category: "source" or x-category: "olap") that let the modal enumerate and filter them directly:
// Instead of:
const connectorsQuery = createRuntimeServiceListConnectorDrivers(...)
$: connectors = $connectorsQuery.data?.connectors?.filter(c => SOURCES.includes(c.name))
// The pattern should be:
import { multiStepFormSchemas } from "./connector-schemas";
$: connectors = Object.entries(multiStepFormSchemas)
.filter(([_, schema]) => schema["x-category"] === "source")
.map(([name, schema]) => ({ name, displayName: schema.title, ... }))This eliminates the dependency on backend connector metadata for UI enumeration and makes the schemas fully authoritative.
ClickHouse special-casing belongs in the schema and renderer, not in parent components 41b0c4e
AddDataForm.svelte has ~30 lines of ClickHouse-specific reactive statements (lines 150-209), and AddDataFormManager.ts has three ClickHouse-specific methods: computeClickhouseState(), getClickhouseDefaults(), and filterClickhouseValues().
The ClickHouse schema already has the machinery to handle all of this:
x-display: "radio"for connector type selectionx-display: "tabs"for parameters/DSN switchingx-visible-iffor conditional field visibilityallOf/if/thenfor conditional requirements and defaults
JSONSchemaFormRenderer interprets these extensions. The parent components should pass the schema to the renderer and handle submission—nothing connector-specific. If the renderer cannot express a ClickHouse requirement, the solution is to extend the renderer or schema vocabulary, not to add branching logic in the parent.
getClickhouseDefaults() duplicates what the schema already expresses 82e78a2
This method (lines 444-477 in AddDataFormManager.ts) computes default values for managed, port, ssl, connector_type, and connection_mode. But these are already default values in the ClickHouse and ClickHouse Cloud schemas:
// clickhousecloud.ts already has:
port: { default: "8443" }
ssl: { default: true }The renderer's job is to apply defaults from the schema. The parent component should not compute them separately.
The two-form architecture (params vs DSN) is legacy that should be removed d5bf870
AddDataForm.svelte creates two separate SuperForms instances (lines 94-124). The usesLegacyTabs variable correctly identifies this as legacy, but the code structure still maintains both forms.
MySQL demonstrates the correct pattern: a single schema with x-display: "tabs" and x-tab-group that switches between field sets. One form, one schema, dynamic field visibility. The separate DSN form instance should be removed.
AddDataFormManager.ts accumulates complexity that schemas were meant to eliminate
This class bridges the legacy backend-properties approach and the new JSON Schema approach. With full commitment to schemas, most of it becomes unnecessary:
- Form IDs/heights → derive from connector name or schema metadata
- Property filtering →
x-stephandles this - ClickHouse state computation → schema + renderer handles it
- SuperForms initialization → direct from schema
The class currently centralizes connector-specific logic rather than eliminating it. A cleaner design would have a thin orchestration layer that passes the schema to the renderer and handles submission.
The synthetic "clickhousecloud" connector in AddDataModal.svelte should not exist #8632 (comment)
Lines 65-84 clone the ClickHouse connector to create a synthetic clickhousecloud connector. Since clickhousecloud.ts schema exists as a first-class schema, it should appear in the connector list directly (via schema enumeration as described above). The synthetic connector object is unnecessary indirection.
propertiesToSchema should be removed 41d6821
This function converts backend ConnectorDriverProperty[] to JSON Schema as a fallback when no schema exists. But schemas exist for all connectors in connector-schemas.ts. If the goal is to eliminate backend property consumption, this fallback defeats the purpose.
If a connector lacks a schema, the form should show an error, not silently fall back to the old approach. Removing this fallback ensures the migration is complete.
Developed in collaboration with Claude Code
|
Fixed in d3d85e9 |
Replace hardcoded connector lists with schema-based metadata: - Multi-step detection now derives from x-category='objectStore' - Explorer mode detection derives from x-category='sqlStore'|'warehouse' - Form heights now driven by x-form-height schema property Remove MULTI_STEP_CONNECTORS, TALL_FORM_CONNECTORS, and associated constants from constants.ts. Add getFormHeight(), isMultiStepConnector(), and isExplorerConnector() helpers to connector-schemas.ts that read from schema metadata instead.
Move form creation out of AddDataFormManager into a dedicated createConnectorForm() factory function in FormValidation.ts. The manager now accepts form stores as parameters instead of creating them, making it a pure orchestration layer that handles step flow, submission routing, and validation coordination. This separation allows form creation to be reused and tested independently of the orchestration logic.
Remove properties and filteredParamsProperties from AddDataFormManager as they were computed but never used - computeYamlPreview retrieves schema fields directly when needed. This completes the simplification of AddDataFormManager to a thin orchestration layer that derives all behavior from JSON schemas.
When S3 sources are rewritten to DuckDB, the model YAML was incorrectly using the original connector instance name (s3) instead of duckdb. This caused the backend to expect a 'path' property but receive 'sql', resulting in "missing property \`path\`" errors. Now when a connector is rewritten to DuckDB, the YAML connector field uses duckdb, while credential access is maintained via create_secrets_from_connectors.
When S3 sources are rewritten to DuckDB, the model YAML was incorrectly using the original connector instance name (s3) instead of duckdb. This caused the backend to expect a 'path' property but receive 'sql', resulting in "missing property \`path\`" errors. Now when a connector is rewritten to DuckDB, the YAML connector field uses duckdb, while credential access is maintained via create_secrets_from_connectors.
Enable users to choose between rill-managed infrastructure and self-hosted ClickHouse Cloud connections, mirroring the same pattern used in the standard ClickHouse connector.
b75f43c to
8918230
Compare










This PR refactors all connector forms to use a unified JSON schema-based validation and rendering system, replacing the previous Yup-based validation approach.
Checklist: