Skip to content

CXH-2381: document Db2 setup, correct the supported-database list, and fix the help link - #150

Open
al-conductorone wants to merge 4 commits into
mainfrom
cxh-2381-baton-sql-document-the-db2-configuration-requirements-and
Open

CXH-2381: document Db2 setup, correct the supported-database list, and fix the help link#150
al-conductorone wants to merge 4 commits into
mainfrom
cxh-2381-baton-sql-document-the-db2-configuration-requirements-and

Conversation

@al-conductorone

Copy link
Copy Markdown
Contributor

Documents how to set up the IBM Db2 database engine for the SQL connector, corrects the list of supported databases so it matches what the connector actually supports, and fixes the connector's help link, which pointed at a dead page.

Add a "Writing a Db2 spec" section to docs/db2.md (wrap every column
reference in string(), alias columns to double-quoted lowercase), notes that
account provisioning is unavailable and group principals can't be represented,
and a "Running the Db2 tests" section with make test-db2 / make vet-db2 and
the raw CGO_CFLAGS/CGO_LDFLAGS + library-path invocation.

Add test-db2 and vet-db2 Makefile targets scoping the CGO flags to the recipe.

Reconcile the engine lists (README, docs/docs-info.md, test/README.md) so they
agree with the dispatch switch in pkg/database/database.go, and drop the
unfilled template sentence from docs-info.md.

Point the connector help URL at the live docs page (/docs/baton/baton-sql);
the old /docs/baton/sql returns 404.

Bump the ci.yaml sync-test action to @v4 and gitignore the root baton-sql
build output.
@linear-code

linear-code Bot commented Sep 2, 2026

Copy link
Copy Markdown

CXH-2381

Comment thread pkg/config/schema.go
Comment thread .github/workflows/ci.yaml
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Connector PR Review: CXH-2381: document Db2 setup, correct the supported-database list, and fix the help link

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 2963cce98f5b.
Review mode: incremental since 3b9c320
View review run

Review Summary

The new commit fde489f is a single change: it drops the /baton-sql and /baton-sql.exe entries from the tracked .gitignore, which addresses @mateoHernandez123's review comment on that file — *.exe two lines up already covered the Windows binary, and the root build artifact belongs in .git/info/exclude. Verified against the current .gitignore (no leftover entries) and against the CI Build baton-sql step, which builds into the workspace and never commits, so nothing regresses. The full PR diff was scanned for security and correctness — the non-doc changes remain the sync-test@v2@v4 bump plus bad-credentials, the test-db2/vet-db2 Makefile targets, and the WithHelpUrl string in pkg/config/schema.go:23 — and no new issues were found; the incremental artifact reported no dropped paths and no truncation. Three earlier suggestions are still open inline and are not re-flagged here: the /docs/baton/baton-sql slug (pkg/config/schema.go:23), the unexplained sync-test major bump (.github/workflows/ci.yaml:44), and the auth-error.sh check still passing vacuously because cmd/baton-sql/main.go uses configSdk.DefineConfiguration + os.Exit(1) instead of config.RunConnector/exit.LogExit (.github/workflows/ci.yaml:50).

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None new — the three prior suggestions listed above remain open as inline comments.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

…h check

Add WordPress (MySQL-based) back to the README engine list so it agrees
with docs/docs-info.md, which still ships it.

Pass bad-credentials: DB_PASSWORD=invalid to sync-test@v4 so the new
auth-error check actually runs; without it the step skips (DB_PASSWORD is
not a BATON_* var, so nothing gets invalidated).

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

Comment thread .github/workflows/ci.yaml
baton-entitlement: 'role:admin:member'
baton-principal: john.smith
baton-principal-type: user
bad-credentials: DB_PASSWORD=invalid

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion: This gets auth-error.sh past the "no credential env vars found" skip, but the check still won't assert anything. cmd/baton-sql/main.go uses the deprecated configSdk.DefineConfiguration + os.Exit(1) pattern rather than config.RunConnector/exit.LogExit, so an invalid DB_PASSWORD exits 1 — which auth-error.sh treats as "not migrated yet, skipping" and passes. Switching main.go to config.RunConnector (available in baton-sdk v0.28.0) would surface gRPC Unauthenticated/PermissionDenied and make this input actually exercise the check.
true

Comment thread docs/db2.md Outdated
Comment thread docs/db2.md Outdated
Comment thread docs/docs-info.md
Comment thread README.md Outdated
- db2.md: reframe the "Writing a Db2 spec" intro. The string()-wrapping
  and lowercase-alias patterns are not Db2-only; Oracle folds unquoted
  identifiers and Redshift needs string() in CEL concatenations. Db2
  just needs both everywhere.
- db2.md: correct the group-provisioning note. grantableTo is
  spec-driven and is not filtered by engine, so a spec that declares
  group still advertises it as grantable; the group grant is dropped at
  ingest, not at grantableTo. Split the two into separate statements.
- docs-info.md: bring the "Example DSN formats" and "Configuration
  Examples" lists in line with the supported-engines list (SAP HANA,
  Vertica, Amazon Redshift, IBM DB2).
- README.md: drop the hand-maintained engine enumeration in Key
  Features that had drifted from the Supported Database Engines list on
  WordPress; link to that list instead.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

Comment thread .gitignore Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

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.

4 participants