Skip to content

connector!: dialect + auth-mode subcommand tree (Postgres)#15

Merged
bradfair merged 1 commit into
mainfrom
feat/connector-dialect-subcommands
Apr 21, 2026
Merged

connector!: dialect + auth-mode subcommand tree (Postgres)#15
bradfair merged 1 commit into
mainfrom
feat/connector-dialect-subcommands

Conversation

@bradfair
Copy link
Copy Markdown
Contributor

@bradfair bradfair commented Apr 21, 2026

Summary

Follow-ups

  • Phase 3c: capture missing dialects via textql-webapp-probe.
  • Phase 3d–e: Snowflake + Databricks subtrees (4 auth modes each).
  • Phase 3f: stub remaining dialects so the tree is discoverable.
  • Phase 3g: dialect-aware connector update + auth-swap subtree.

Summary by CodeRabbit

  • New Features
    • Reorganized connector creation commands into a hierarchical structure organized by database type.
    • Introduced Postgres connector creation workflow with password-based authentication support.
    • Updated command interface for improved clarity and extensibility.

Rearchitects `ana connector create` into a two-level Group tree so new
dialects and auth modes land as file additions rather than growing a
conditional `--type`/`--auth` flag matrix. Postgres (one auth mode today
— password) is migrated as the first dialect; Snowflake/Databricks/etc.
will follow as probes capture their wire shapes.

- newCreateGroup → dialect selector Group ("create" child of connector).
- newPostgresCreateGroup → Postgres dialect Group owning common flags
  --name and --ssl via cli.Group.Flags (Phase 2 inheritable flags).
- postgresPasswordCmd → `password` auth-mode leaf. Implements cli.Flagger
  so --help renders own + ancestor flags. Reads --name/--ssl through
  pointer fields bound in newPostgresCreateGroup's closure.
- resolvePassword stays in create.go; update.go still uses it.

User-visible change: `ana connector create --type postgres --name X ...`
becomes `ana connector create postgres password --name X ...`. Scoped `!`
for the SemVer major bump since this is a breaking flag-shape change.

Docs:
- README: connector-create subcommand shape noted in Getting started.
- README: Windows SmartScreen section rewritten to distinguish signed
  release binaries (Azure Trusted Signing via sign-windows job) from
  unsigned source builds. Closes #8.
- docs/cli-readiness.md: example command updated to new shape.

Tests: 100% cov on ./internal/connector/. All create cases route through
newCreateGroup so the ancestor-flag plumbing (Postgres Group → leaf) is
exercised the same way real dispatch does.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9863ed17-4d02-4306-b870-ffc6b688dcfa

📥 Commits

Reviewing files that changed from the base of the PR and between 33bb9c3 and 6e62586.

⛔ Files ignored due to path filters (3)
  • README.md is excluded by !**/*.md
  • docs/cli-readiness.md is excluded by !**/docs/**, !**/*.md
  • internal/connector/CLAUDE.md is excluded by !**/*.md
📒 Files selected for processing (6)
  • internal/connector/connector.go
  • internal/connector/connector_test.go
  • internal/connector/create.go
  • internal/connector/create_postgres.go
  • internal/connector/create_postgres_password_test.go
  • internal/connector/create_test.go
💤 Files with no reviewable changes (2)
  • internal/connector/connector_test.go
  • internal/connector/create_test.go

📝 Walkthrough

Walkthrough

The connector create command architecture is refactored from a flat single command into a hierarchical CLI group structure. The old createCmd is removed and replaced with newCreateGroup(), which nests dialect-specific subcommands. A Postgres dialect handler is introduced in a new file, and tests are migrated from the deprecated implementation to the new Postgres-specific test suite.

Changes

Cohort / File(s) Summary
Command routing
internal/connector/connector.go, internal/connector/connector_test.go
Updated "create" subcommand dispatch to use newCreateGroup(deps) instead of flat createCmd; removed "create" from help text test cases.
Old command removal
internal/connector/create.go
Removed createCmd struct and its methods (Help, Run); added newCreateGroup(deps) factory that builds hierarchical CLI subtree nesting Postgres dialect group; updated resolvePassword documentation.
Postgres dialect implementation
internal/connector/create_postgres.go
New file introducing newPostgresCreateGroup(deps) parent group and postgresPasswordCmd leaf command. Implements Postgres-specific flag parsing (--host, --port, --user, --database, --password/--password-stdin), required-field validation (rejecting empty strings, enforcing port range 1–65535), password resolution, connector request construction with type "POSTGRES" and postgresSpec, and API submission via deps.Unary.
Postgres tests
internal/connector/create_postgres_password_test.go
New test file covering happy-path connector creation, flag inheritance/wiring (e.g., --ssl into request), JSON output mode, stdin password reading, validation errors (missing/empty flags, invalid ports), error propagation (stdin read, IO write, RPC, decode failures), and resolvePassword unit tests (nil reader handling, whitespace preservation, line-terminator stripping).
Old test removal
internal/connector/create_test.go
Deleted entire test suite for flat createCmd, including coverage of flag validation, password stdin handling, JSON mode, and error scenarios (now migrated/adapted in create_postgres_password_test.go).

Sequence Diagram

sequenceDiagram
    participant Client as CLI Caller
    participant CreateGroup as Create Group
    participant PostgresGroup as Postgres Group
    participant PasswordCmd as Password Command
    participant Flags as Flag Parser
    participant API as API (Unary)
    participant IO as Output Handler

    Client->>CreateGroup: Run(args)
    CreateGroup->>PostgresGroup: Dispatch "postgres"
    PostgresGroup->>PasswordCmd: Dispatch "password"
    PasswordCmd->>Flags: Parse args with inherited flags
    Flags->>Flags: Register inherited (--name, --ssl)
    Flags->>Flags: Register own flags (--host, --port, --user, --database, --password*)
    Flags->>Flags: Validate required fields & ranges
    PasswordCmd->>PasswordCmd: Resolve password from --password or stdin
    PasswordCmd->>API: POST /CreateConnector with postgresSpec
    API->>PasswordCmd: Response (connectorId, name, connectorType)
    PasswordCmd->>IO: Render connector details to stdout
    IO->>Client: Success output
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Possibly related issues

  • connector create: support snowflake + databricks dialects #1: The refactoring of connector create into a dialect-based hierarchical subcommand tree (via newCreateGroup and newPostgresCreateGroup) establishes the architectural foundation for registering additional database dialects (Snowflake, Databricks, etc.) as sibling Postgres groups.

Possibly related PRs

  • refactor(connector): extract shared wire types #12: The new Postgres create command uses connector wire types (createReq, postgresSpec, createResp, etc.) that are centralized in an upcoming types refactor, representing a direct code-level dependency.
  • chore(main): release 0.3.0 #13: Both PRs modify the core connector create implementation, with this PR replacing the flat command structure while the other refactors shared CLI patterns—changes that may interact during integration.

Poem

🐰 A create command now sprouts dialect flowers,
Postgres blooms first beneath hierarchical bowers,
Flags dance together through nested command towers,
Testing paints new patterns with validation powers, 🌸
Future dialects await their flowering hours!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.31% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: refactoring 'ana connector create' into a dialect and auth-mode subcommand tree with Postgres as the first implementation. The breaking change indicator (!) is appropriate.
Description check ✅ Passed The PR description is comprehensive, covering the rearchitecture, breaking changes, related issue fixes, and follow-up phases. It aligns well with the description template by explaining what changed and why.
Linked Issues check ✅ Passed The PR fulfills the primary objective from #8 by rewriting the Windows SmartScreen README section to document that release binaries are Authenticode-signed via Azure Trusted Signing, distinguishing them from unsigned source builds.
Out of Scope Changes check ✅ Passed All code changes are directly aligned with the PR objectives: the connector create refactoring into a dialect/auth-mode tree and the README update for issue #8. No unrelated changes are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/connector-dialect-subcommands

Comment @coderabbitai help to get the list of available commands and usage tips.

@bradfair bradfair added this pull request to the merge queue Apr 21, 2026
Merged via the queue into main with commit 6edf0bd Apr 21, 2026
10 checks passed
@bradfair bradfair deleted the feat/connector-dialect-subcommands branch April 21, 2026 17:29
@hpt-bot hpt-bot Bot mentioned this pull request Apr 21, 2026
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.

docs: README incorrectly states Windows binary is unsigned

1 participant