Skip to content

Connectors JSON Schema#8632

Merged
lovincyrus merged 108 commits intomainfrom
cyrus/all-connectors-json-schema
Feb 3, 2026
Merged

Connectors JSON Schema#8632
lovincyrus merged 108 commits intomainfrom
cyrus/all-connectors-json-schema

Conversation

@lovincyrus
Copy link
Contributor

@lovincyrus lovincyrus commented Jan 13, 2026

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:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@lovincyrus lovincyrus self-assigned this Jan 13, 2026
@lovincyrus lovincyrus force-pushed the cyrus/all-connectors-json-schema branch from 274c8cd to 45c3e9f Compare January 13, 2026 22:59
@lovincyrus lovincyrus requested a review from royendo January 14, 2026 18:54
@royendo
Copy link
Contributor

royendo commented Jan 14, 2026

bug:
when athen errors, typing doesnt clear error
Screenshot 2026-01-14 at 15 42 13

nit:
bigquery should not have project-id as optional, errors if its not there
also a Title like AWS Access Key ID in athena would be great "GCP Credential", "Service Account JSON", etc.
Screenshot 2026-01-14 at 15 42 16

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?
same with adding two step here, we're already in the .ts files (but thats just my Product opinion, defer to @ericpgreen2 for code work)

Copy link
Contributor

@royendo royendo left a comment

Choose a reason for hiding this comment

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

see above :)

@lovincyrus
Copy link
Contributor Author

bug: when athen errors, typing doesnt clear error Screenshot 2026-01-14 at 15 42 13

nit: bigquery should not have project-id as optional, errors if its not there also a Title like AWS Access Key ID in athena would be great "GCP Credential", "Service Account JSON", etc. Screenshot 2026-01-14 at 15 42 16

Addressed both! The main focus of this branch is to land the schema-driven design for all connectors. I plan to tackle others when the base branch is clean.

@lovincyrus lovincyrus requested a review from royendo January 14, 2026 23:20
@lovincyrus lovincyrus marked this pull request as ready for review January 14, 2026 23:21
Copy link
Contributor

@ericpgreen2 ericpgreen2 left a 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" or x-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 from ConnectorDriverProperty[]
  • AddDataFormManager.ts:164-191, 430-441 - derives properties, DSN detection, initial values
  • MultiStepConnectorFlow.svelte:59-60, 77 - uses sourceProperties/configProperties for 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" with x-tab-group). This unblocks ClickHouse and other connectors with dual input modes.

  • ClickHouse rendering through JSONSchemaFormRenderer - Delete ClickhouseFormRenderer.svelte once 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 with x-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

Copy link
Contributor

@royendo royendo left a 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.
Image
  • Going back doesnt remove bucket
Image
  • style added in copy paste
Image
  • connector info added to model
Image
  • Connection_mode added to YAML preview (not actual file) (snowflake)
connection_mode: dsn
  • create_secrets_from_connectors: https showing 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)

@lovincyrus
Copy link
Contributor Author

  • If possible, move CHCloud to its own button

Sorry, what do you mean by moving CHCloud to its own button? @royendo

@royendo
Copy link
Contributor

royendo commented Jan 21, 2026

@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
https://www.figma.com/design/I79srwZXL0i6dthpE1wzMj/Roy-s-playgronud?node-id=355-5044&t=L0MlBAU3Ik3RGvrd-1

this will also simplify the CH button UI, lots of radio icons :)

@royendo
Copy link
Contributor

royendo commented Jan 21, 2026

QA:

Screenshot 2026-01-21 at 14 22 59 Screenshot 2026-01-21 at 14 23 33
  • BQ hint, "or pasted", is this actually a method?
  • commented out reference missing from models
# Model YAML
# Reference documentation: https://docs.rilldata.com/reference/project-files/models

2026-01-21T14:34:32.612 WARN    Reconcile failed     {"name": "clickhouse", "type": "Connector", "elapsed": "18ms", "error": "failed to start clickhouse: fork/exec /Users/royendo/.rill/clickhouse/25.6.12.10/clickhouse: permission denied"}

but nothing surfaces to UI. (had to make cli to get the error)

Copy link
Contributor

@royendo royendo left a comment

Choose a reason for hiding this comment

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

see comment

@lovincyrus
Copy link
Contributor Author

  • now model preview is gone completely

The right sidebar was never supposed to be there from the Figma mocks.

@lovincyrus
Copy link
Contributor Author

lovincyrus commented Jan 21, 2026

  • style added in copy paste
Image

Monospace styling was added to the SQL input field based on this feedback.

@lovincyrus
Copy link
Contributor Author

[ ] BQ hint, "or pasted", is this actually a method?

Sorry, I don't understand this.

@lovincyrus
Copy link
Contributor Author

lovincyrus commented Jan 21, 2026

  • creating a second clickhouse managed connector results in error in logs
2026-01-21T14:34:32.612 WARN    Reconcile failed     {"name": "clickhouse", "type": "Connector", "elapsed": "18ms", "error": "failed to start clickhouse: fork/exec /Users/royendo/.rill/clickhouse/25.6.12.10/clickhouse: permission denied"}

but nothing surfaces to UI. (had to make cli to get the error)

Out of scope. Let's track this elsewhere. This branch is for the connectors JSON schema files.

@lovincyrus
Copy link
Contributor Author

@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 https://www.figma.com/design/I79srwZXL0i6dthpE1wzMj/Roy-s-playgronud?node-id=355-5044&t=L0MlBAU3Ik3RGvrd-1

this will also simplify the CH button UI, lots of radio icons :)

Done!

Copy link
Contributor

@ericpgreen2 ericpgreen2 left a 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 selection
  • x-display: "tabs" for parameters/DSN switching
  • x-visible-if for conditional field visibility
  • allOf/if/then for 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-step handles 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

@royendo
Copy link
Contributor

royendo commented Jan 22, 2026

nit: PrivateKey placeholder not showing up:
Screenshot 2026-01-22 at 14 32 56

@lovincyrus
Copy link
Contributor Author

nit: PrivateKey placeholder not showing up: Screenshot 2026-01-22 at 14 32 56

Fixed in d3d85e9

…extracted variables

- Remove duplicate JSDoc comment on computeYamlPreview
- Extract explanatory variables in getPrimaryButtonLabel (isOnConnectorStep, isOnSourceOrExplorerStep)
- Add comments clarifying each button label branch
- Extract three key variables in makeOnUpdate for clarity:
  - isOnSourceOrExplorerStep: reduces conditional duplication
  - isOnConnectorStep: makes connector step checks explicit
  - isPublicAuth: clarifies authentication method checks
- Extract submitConnectorStepAndAdvance private method to reduce nesting
- Add section comments throughout makeOnUpdate explaining validation flow and submission logic
royendo added a commit that referenced this pull request Jan 31, 2026
royendo added a commit that referenced this pull request Jan 31, 2026
- Add openExplorerForConnector function to open Data Explorer directly
- Add + button to ConnectorEntry for OLAP connectors to open Data Explorer
- Integrate explorer step into AddDataForm after PR #8632 merge
- Show AddDataExplorerStep when in explorer mode, hide footer/sidebar

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@royendo
Copy link
Contributor

royendo commented Feb 2, 2026

Screenshot 2026-02-02 at 13 01 26 Screenshot 2026-02-02 at 13 01 33 Screenshot 2026-02-02 at 13 03 49
type: model
materialize: true

connector: bigquery

sql: select * from rilldata.andrew_test.big_numeric_test

dev:
  sql: select * from rilldata.andrew_test.big_numeric_test limit 10000
Screenshot 2026-02-02 at 13 08 38
  • Font is weird in SQL still
Screenshot 2026-02-02 at 13 08 28

@lovincyrus
Copy link
Contributor Author

  • placeholder text still is not matching
Screenshot 2026-02-02 at 13 03 49

I'm not sure which placeholder text you're referring to. I tested the parameters form and it worked fine. @royendo

@lovincyrus
Copy link
Contributor Author

lovincyrus commented Feb 2, 2026

  • Save Anyways doesn't show always
Screenshot 2026-02-02 at 13 01 26

This is totally unrelated to Save Anyways code path. The reason you don't see the button is because the validation rule caught the invalid pattern string, hence "Must be an S3 URI (e.g., s3://bucket/path/)"

CleanShot.2026-02-02.at.11.10.56.mp4

@lovincyrus
Copy link
Contributor Author

  • weird issue with BQ timing
Screenshot 2026-02-02 at 13 08 38

I am unclear what you're referring to.

@lovincyrus
Copy link
Contributor Author

  • Generated YAML doesnt have URL
type: model
materialize: true

connector: bigquery

sql: select * from rilldata.andrew_test.big_numeric_test

dev:
  sql: select * from rilldata.andrew_test.big_numeric_test limit 10000

The URL is attached to the top of the YAML.

CleanShot 2026-02-02 at 11 20 59@2x

@royendo
Copy link
Contributor

royendo commented Feb 2, 2026

  • the CSS differs on Private Key place holder and other forms, these should be standard
  • BQ Data explorer shows error
  • strange, it didnt for me, maybe it was generated from data explorer?
  • For Athena form, i hit submit and the error came up. i get the logic, can keep as is for now.

@royendo
Copy link
Contributor

royendo commented Feb 3, 2026

Screenshot 2026-02-03 at 11 43 48
  • connection_mode should not be here, similarly
Screenshot 2026-02-03 at 11 47 32 Screenshot 2026-02-03 at 11 49 16

@royendo
Copy link
Contributor

royendo commented Feb 3, 2026

@lovincyrus still looking but giving you a few to start to fix

@lovincyrus
Copy link
Contributor Author

  • these are still not the same css
Screenshot 2026-02-03 at 11 43 48

As mentioned, the change was implemented in this #8725. Consider this done.

When building the source/explorer YAML preview, connector properties should be
excluded. The filter incorrectly excluded internal fields (x-ui-only: true)
from the connector property key set, causing connection_mode to slip through
and appear in the model preview.

Removing the filter ensures all connector-step fields—including internal
UI-only fields like connection_mode—are properly excluded from the
source/explorer preview.
Copy link
Contributor

@royendo royendo left a comment

Choose a reason for hiding this comment

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

LGTM, Great work Cyrus

@lovincyrus lovincyrus merged commit 4a34498 into main Feb 3, 2026
11 checks passed
@lovincyrus lovincyrus deleted the cyrus/all-connectors-json-schema branch February 3, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants