Merge databricks:main into dbt-labs:main - #6
Closed
serramatutu wants to merge 113 commits into
Closed
Conversation
This PR fixes an issue where the driver discards the `context.Context` during polling, making it impossible to use authentication mechanisms (like Azure OBO) that rely on passing credentials via the context. Reference: - #288 - grafana/grafana#112955 Signed-off-by: Diego Giagio <diego.giagio@grafana.com>
…nd opt-in configuration (#319) ## Summary This PR implements Phases 4-5 of the telemetry system for the Databricks SQL Go driver. **Stack:** Part 1 of 2 - This PR: PECOBLR-1143 (Phases 4-5) - Next: PECOBLR-1381-1382 (Phases 6-7) --- ## Phase 4: Export Infrastructure ✅ **New file: `exporter.go` (192 lines)** - ✅ `telemetryExporter` with circuit breaker integration - ✅ HTTP POST to `/api/2.0/telemetry-ext` endpoint - ✅ Exponential backoff retry (100ms base, 3 retries max) - ✅ Tag filtering via `shouldExportToDatabricks()` - ✅ JSON serialization (`telemetryPayload`, `exportedMetric`) - ✅ Comprehensive error swallowing - ✅ Support for HTTP and HTTPS URLs **New file: `exporter_test.go` (448 lines)** - ✅ 17 comprehensive tests with mock HTTP server - ✅ Success scenarios, retry logic, circuit breaker - ✅ Tag filtering, error swallowing, exponential backoff - ✅ Context cancellation, 4xx/5xx handling --- ## Phase 5: Opt-In Configuration Integration ✅ **Updated: `config.go` (+48 lines)** - ✅ `isTelemetryEnabled()` with 5-level priority logic - ✅ Integration with `featureFlagCache` - ✅ Error handling with safe fallbacks **Updated: `config_test.go` (+230 lines)** - ✅ 8 tests for all 5 priority levels - ✅ Server error scenarios, unreachable hosts **Priority Logic:** 1. `forceEnableTelemetry=true` → always enabled 2. `enableTelemetry=false` → always disabled 3. `enableTelemetry=true` + server feature flag check 4. Server-side feature flag only (default) 5. Default disabled (fail-safe) --- ## Changes **Total:** +976 insertions, -57 deletions --- ## Testing **All 70+ tests passing** ✅ (2.017s) - Circuit Breaker: 15 tests ✓ - Config & Opt-In: 19 tests ✓ - Exporter: 17 tests ✓ - Feature Flags: 12 tests ✓ - Manager: 9 tests ✓ - Tags: 7 tests ✓ --- ## Related Issues - Implements: PECOBLR-1143 (Phases 4-5) - Enables: PECOBLR-1381 (Phase 6) - Enables: PECOBLR-1382 (Phase 7) --- ## Checklist - [x] Implements Phase 4 export infrastructure - [x] Implements Phase 5 opt-in configuration - [x] Comprehensive unit tests - [x] All tests passing - [x] DESIGN.md checklist updated - [x] No breaking changes --------- Signed-off-by: samikshya-chand_data <samikshya.chand@databricks.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Two comprehensive examples demonstrating token provider usage: 1. token_federation: Simple external token provider with federation 2. browser_oauth_federation: Full browser OAuth flow with automatic token exchange Both examples show real-world integration patterns for custom authentication. --------- Co-authored-by: Claude <noreply@anthropic.com>
New version → v1.10.0 (new features warrant a minor bump). This release has a bunch of bug fixes too
## Summary - `execStagingOperation` creates a `Rows` object via `rows.NewRows()` to read staging operation metadata (presigned URL, headers, local file path) but never calls `row.Close()` - This leaks the `Rows` object and its `RowScanner` resources until GC collects them - Add `defer row.Close()` to ensure proper cleanup after reading the staging metadata Note: the server-side operation is already closed by `ExecContext` (lines 122-133), so this is a client-side resource leak rather than a server-side operation leak. ## Test plan - [x] Existing `TestConn_execStagingOperation` tests pass - [x] `TestWorkflowExample` e2e test passes - [x] `go build ./...` compiles cleanly Related: #275 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## Summary - Pin all GitHub Actions to verified commit SHAs (0/6 were pinned before) - Add explicit least-privilege `permissions:` blocks to all workflows - Replace third-party `tisonkun/actions-dco` action with inline DCO check script (ported from databricks-jdbc) - Switch runners from `ubuntu-latest` to `databricks-protected-runner-group` - Replace `curl | sh` golangci-lint install with `go install` in Makefile - Add Dependabot configuration for Go modules and GitHub Actions ## Security findings addressed Addresses findings from CI/CD supply chain security analysis: - **Critical**: All GitHub Actions pinned to mutable tags → now SHA-pinned - **Critical**: No permissions blocks → explicit least-privilege scoping - **High**: Third-party DCO action from individual publisher → inline script - **High**: curl|sh without checksum → go install - **Medium**: No Dependabot → automated dependency updates ## Test plan - [ ] Verify lint job runs successfully with golangci-lint-action - [ ] Verify DCO check correctly passes for signed commits - [ ] Verify DCO check correctly fails for unsigned commits - [ ] Verify build-and-test job completes on protected runner group - [ ] Verify Dependabot creates initial update PRs This pull request was AI-assisted by Isaac. Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…340) ## Summary Follow-up to #329 — the squash merge lost the second commit with these fixes. - Switch all workflows back to `ubuntu-latest` (GitHub-hosted runners) to avoid executing fork PR code on Databricks infrastructure - Fix DCO check to not checkout attacker-controlled fork code — uses the default `pull_request` merge ref (which already contains all PR commits) instead of explicitly checking out the fork's `head.ref`/`head.repo` - Move `BASE_SHA`/`HEAD_SHA` to `env:` vars to prevent script injection via crafted commit SHAs - Add CODEOWNERS requiring `@databricks/eng-oss-sql-driver` review for `.github/` changes - Add SECURITY.md vulnerability disclosure policy ## Security findings addressed - **High**: Self-hosted runners on public repo allow fork PRs to execute on org infrastructure → `ubuntu-latest` - **High**: DCO check explicitly checks out attacker-controlled fork code on runner → default merge ref - **Medium**: No CODEOWNERS for `.github/workflows/` → added - **Medium**: No SECURITY.md → added ## Test plan - [ ] Verify DCO check passes for signed commits - [ ] Verify DCO check fails for unsigned commits - [ ] Verify CI runs on GitHub-hosted runners This pull request was AI-assisted by Isaac. Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…n, Aggregation & Driver Integration (#320) ## Summary This **stacked PR** builds on #319 and implements Phases 6-7 of the telemetry system, completing the full pipeline. **Stack:** Part 2 of 2 - Base: #319 (PECOBLR-1143 - Phases 4-5) - This PR: PECOBLR-1381 + PECOBLR-1382 (Phases 6-7) --- ## Phase 6: Metric Collection & Aggregation ✅ ### New Files **`errors.go` (108 lines)** - ✅ `isTerminalError()` - Non-retryable error detection - ✅ `classifyError()` - Error categorization - ✅ HTTP error handling utilities **`interceptor.go` (146 lines)** - ✅ `BeforeExecute()` / `AfterExecute()` hooks - ✅ Context-based metric tracking - ✅ Latency measurement - ✅ Tag collection - ✅ Error swallowing **`aggregator.go` (242 lines)** - ✅ Statement-level aggregation - ✅ Batch processing (size: 100) - ✅ Background flush (interval: 5s) - ✅ Thread-safe operations - ✅ Immediate flush for terminal errors **`client.go` (updated)** - ✅ Full pipeline integration - ✅ Graceful shutdown --- ## Phase 7: Driver Integration ✅ ### Configuration Support **`internal/config/config.go` (+18 lines)** - ✅ `EnableTelemetry` field - ✅ `ForceEnableTelemetry` field - ✅ DSN parameter parsing - ✅ `DeepCopy()` support ### Connection Integration **`connection.go`, `connector.go` (+20 lines)** - ✅ Telemetry field in `conn` struct - ✅ Initialization in `Connect()` - ✅ Cleanup in `Close()` ### Helper Module **`driver_integration.go` (59 lines)** - ✅ `InitializeForConnection()` - Setup - ✅ `ReleaseForConnection()` - Cleanup - ✅ Feature flag checking - ✅ Resource management --- ## Integration Flow ``` DSN: "host:port/path?enableTelemetry=true" ↓ connector.Connect() ↓ telemetry.InitializeForConnection() ├─→ Feature flag check (5-level priority) ├─→ Get/Create telemetryClient (per host) └─→ Create Interceptor (per connection) ↓ conn.telemetry = Interceptor ↓ conn.Close() ├─→ Flush pending metrics └─→ Release resources ``` --- ## Changes **Total:** +1,073 insertions, -48 deletions (13 files) ### Phase 6: - `telemetry/errors.go` (108 lines) - NEW - `telemetry/interceptor.go` (146 lines) - NEW - `telemetry/aggregator.go` (242 lines) - NEW - `telemetry/client.go` (+27/-9) - MODIFIED ### Phase 7: - `telemetry/driver_integration.go` (59 lines) - NEW - `internal/config/config.go` (+18) - MODIFIED - `connection.go` (+10) - MODIFIED - `connector.go` (+10) - MODIFIED - `telemetry/DESIGN.md` - MODIFIED --- ## Testing **All tests passing** ✅ - ✅ 70+ telemetry tests (2.018s) - ✅ No breaking changes - ✅ Compilation verified - ✅ Thread-safety verified --- ## Usage Example ```go // Enable telemetry via DSN dsn := "host:443/sql/1.0?enableTelemetry=true" db, _ := sql.Open("databricks", dsn) // Or force enable dsn := "host:443/sql/1.0?forceEnableTelemetry=true" ``` --- ## Related Issues - Builds on: #319 (PECOBLR-1143) - Implements: PECOBLR-1381 (Phase 6) ✅ - Implements: PECOBLR-1382 (Phase 7) ✅ --- ## Checklist - [x] Phase 6: Collection & aggregation - [x] Phase 7: Driver integration - [x] Configuration support - [x] Resource management - [x] All tests passing - [x] No breaking changes - [x] DESIGN.md updated --------- Signed-off-by: samikshya-chand_data <samikshya.chand@databricks.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
## Summary - Adds per-statement query tag support via `driverctx.NewContextWithQueryTags`, allowing users to attach query tags to individual SQL statements through context - Tags are serialized into `TExecuteStatementReq.ConfOverlay["query_tags"]`, consistent with the Python ([#736](databricks/databricks-sql-python#736)) and NodeJS ([#339](databricks/databricks-sql-nodejs#339)) connector implementations - Previously only session-level query tags were supported (set once via `WithSessionParams` at connection time) ## Usage ```go ctx := driverctx.NewContextWithQueryTags(context.Background(), map[string]string{ "team": "data-eng", "app": "etl-pipeline", }) rows, err := db.QueryContext(ctx, "SELECT * FROM table") ``` ## Changes | File | Description | |------|-------------| | `driverctx/ctx.go` | `NewContextWithQueryTags`, `QueryTagsFromContext`, propagation in `NewContextFromBackground` | | `query_tags.go` *(new)* | `SerializeQueryTags` — map to wire format with escaping | | `connection.go` | Read tags from context → serialize → set `ConfOverlay["query_tags"]` | | `driverctx/ctx_test.go` | 5 tests for context helpers | | `query_tags_test.go` *(new)* | 13 tests for serialization (escaping, edge cases) | | `connection_test.go` | 6 integration tests verifying ConfOverlay behavior | | `examples/query_tags/main.go` | Updated with session + statement-level examples | ## Test plan - [x] Unit tests for `SerializeQueryTags` covering nil, empty, single/multi tags, escaping of `\`, `:`, `,` in values and keys - [x] Unit tests for `NewContextWithQueryTags` / `QueryTagsFromContext` including nil context, missing key, timeout preservation, background propagation - [x] Integration tests verifying `ConfOverlay["query_tags"]` is correctly set (or absent) in captured `TExecuteStatementReq` - [ ] Verify existing tests still pass (CI) This pull request was AI-assisted by Isaac. --------- Signed-off-by: Jooho Yeo <jooho.yeo@databricks.com> Co-authored-by: Jooho Yeo <jooho.yeo@databricks.com> Co-authored-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…#321) ## Summary This **stacked PR** builds on #320 and adds statement execution hooks to complete end-to-end telemetry collection. **Stack:** Part 3 of 3 - Base: #319 (PECOBLR-1143 - Phases 4-5) - Previous: #320 (PECOBLR-1381-1382 - Phases 6-7) - This PR: PECOBLR-1383 (Statement execution hooks) --- ## Changes ### Exported Methods for Driver Integration **`telemetry/interceptor.go`** - ✅ Exported `BeforeExecute()` - starts metric tracking for a statement - ✅ Exported `AfterExecute()` - records metric with timing and error info - ✅ Exported `AddTag()` - adds tags to current metric context - ✅ Exported `CompleteStatement()` - marks statement complete and flushes ### Statement Execution Hooks **`connection.go`** - ✅ Added hooks to `QueryContext()`: - Calls `BeforeExecute()` with statement ID from operation handle GUID - Uses defer to call `AfterExecute()` and `CompleteStatement()` - ✅ Added hooks to `ExecContext()`: - Calls `BeforeExecute()` with statement ID - Proper error handling (includes stagingErr) - Uses defer to call `AfterExecute()` and `CompleteStatement()` ### Documentation **`telemetry/DESIGN.md`** - ✅ Updated Phase 6 to mark as completed - ✅ Added statement execution hooks to Phase 7 checklist --- ## Integration Flow ``` Connection.QueryContext() ↓ BeforeExecute(statementID) → creates metricContext with startTime ↓ [Statement Execution] ↓ AfterExecute(err) → records metric with latency and error ↓ CompleteStatement(statementID, failed) → flushes aggregated metrics ``` --- ## Testing **All tests passing** ✅ - ✅ 99 telemetry tests (2.018s) - ✅ All driver tests (58.576s) - ✅ No breaking changes - ✅ Telemetry properly disabled when not configured --- ## End-to-End Telemetry With this PR, the telemetry system is **fully functional end-to-end**: 1. ✅ **Collection** - Metrics collected from QueryContext/ExecContext 2. ✅ **Aggregation** - Statement-level aggregation with batching 3. ✅ **Circuit Breaker** - Protection against failing endpoints 4. ✅ **Export** - HTTP POST with retry and exponential backoff 5. ✅ **Feature Flags** - Server-side control with 5-level priority 6. ✅ **Resource Management** - Per-host clients with reference counting --- ## Related Issues - Builds on: #320 (PECOBLR-1381-1382) - Implements: PECOBLR-1383 (Statement execution hooks) ✅ --- ## Checklist - [x] Export interceptor methods for driver use - [x] Add hooks to QueryContext - [x] Add hooks to ExecContext - [x] Update DESIGN.md checklist - [x] All tests passing - [x] No breaking changes --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
## Summary - Adds `internal/agent` package that detects 7 AI coding agents (Claude Code, Cursor, Gemini CLI, Cline, Codex, OpenCode, Antigravity) by checking well-known environment variables they set in spawned shell processes - Integrates detection into `InitThriftClient` to append `agent/<product>` to the User-Agent header - Uses exactly-one detection rule: if zero or multiple agent env vars are set, no agent is attributed (avoids ambiguity) ## Approach Mirrors the implementation in [databricks/cli#4287](databricks/cli#4287) and aligns with the latest agent list in [`libs/agent/agent.go`](https://github.com/databricks/cli/blob/main/libs/agent/agent.go#L35). | Agent | Product String | Environment Variable | |-------|---------------|---------------------| | Google Antigravity | `antigravity` | `ANTIGRAVITY_AGENT` | | Claude Code | `claude-code` | `CLAUDECODE` | | Cline | `cline` | `CLINE_ACTIVE` | | OpenAI Codex | `codex` | `CODEX_CI` | | Cursor | `cursor` | `CURSOR_AGENT` | | Gemini CLI | `gemini-cli` | `GEMINI_CLI` | | OpenCode | `opencode` | `OPENCODE` | Adding a new agent requires only a new constant and a new entry in `knownAgents`. ## Changes - **New**: `internal/agent/agent.go` — environment-variable-based agent detection with injectable env lookup for testability - **New**: `internal/agent/agent_test.go` — 11 test cases covering all agents, no agent, multiple agents, empty values, and real `os.Getenv` - **Modified**: `internal/client/client.go` — calls `agent.Detect()` when building User-Agent in `InitThriftClient` ## Test plan - [x] `internal/agent` — 11 tests pass - [x] `internal/client` — all existing tests continue to pass - [x] Manual: verified User-Agent contains `agent/claude-code` when run from Claude Code via `GODEBUG=http2debug=2` ``` http2: Transport encoding header "user-agent" = "godatabrickssqlconnector/1.10.0 agent/claude-code" ``` - [x] Executed `SELECT 1` successfully against dogfood warehouse with the new header 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## Summary This **final stacked PR** completes the telemetry implementation with comprehensive testing, launch documentation, and user-facing documentation for all remaining phases (8-10). **Stack:** Part 4 of 4 (Final) - Base: #319 (PECOBLR-1143 - Phases 4-5) - Previous: #320 (PECOBLR-1381-1382 - Phases 6-7) - Previous: #321 (PECOBLR-1383 - Execution hooks) - This PR: PECOBLR-1384 (Phases 8-10) ✅ **TELEMETRY COMPLETE** --- ## Phase 8: Testing & Validation ✅ ### Benchmark Tests (`benchmark_test.go` - 392 lines) **Performance Benchmarks:** - `BenchmarkInterceptor_Overhead_Enabled`: 36μs/op (< 0.1% overhead) - `BenchmarkInterceptor_Overhead_Disabled`: 3.8ns/op (negligible) - `BenchmarkAggregator_RecordMetric`: Aggregation performance - `BenchmarkExporter_Export`: Export performance - `BenchmarkConcurrentConnections_PerHostSharing`: Per-host sharing efficiency - `BenchmarkCircuitBreaker_Execute`: Circuit breaker overhead **Load & Integration Tests:** - `TestLoadTesting_ConcurrentConnections`: 100+ concurrent connections - `TestGracefulShutdown_ReferenceCountingCleanup`: Reference counting validation - `TestGracefulShutdown_FinalFlush`: Final flush on shutdown ### Integration Tests (`integration_test.go` - 356 lines) - `TestIntegration_EndToEnd_WithCircuitBreaker`: Complete flow validation - `TestIntegration_CircuitBreakerOpening`: Circuit breaker behavior under failures - `TestIntegration_OptInPriority_ForceEnable`: forceEnableTelemetry verification - `TestIntegration_OptInPriority_ExplicitOptOut`: enableTelemetry=false verification - `TestIntegration_PrivacyCompliance_NoQueryText`: No sensitive data collected - `TestIntegration_TagFiltering`: Tag allowlist enforcement **Results:** - ✅ All 115+ tests passing - ✅ Performance overhead < 1% when enabled - ✅ Negligible overhead when disabled - ✅ Circuit breaker protects against failures - ✅ Per-host client sharing prevents rate limiting - ✅ Privacy compliance verified --- ## Phase 9: Partial Launch Preparation ✅ ### Launch Documentation (`LAUNCH.md` - 360 lines) **Phased Rollout Strategy:** 1. **Phase 1: Internal Testing** (2-4 weeks) - `forceEnableTelemetry=true` - Internal users and dev teams - Validate reliability and performance 2. **Phase 2: Beta Opt-In** (4-8 weeks) - `enableTelemetry=true` - Early adopter customers - Gather feedback and metrics 3. **Phase 3: Controlled Rollout** (6-8 weeks) - Server-side feature flag - Gradual rollout: 5% → 25% → 50% → 100% - Monitor health metrics **Configuration Priority:** 1. forceEnableTelemetry=true (internal only) 2. enableTelemetry=false (explicit opt-out) 3. enableTelemetry=true + server flag (user opt-in) 4. Server feature flag only (default) 5. Default disabled **Monitoring & Alerting:** - Performance metrics (latency, memory, CPU) - Reliability metrics (error rate, circuit breaker) - Business metrics (feature adoption, error patterns) - Alert thresholds and escalation procedures **Rollback Procedures:** - Server-side flag disable (immediate) - Client-side workaround (enableTelemetry=false) - Communication plan (internal/external) --- ## Phase 10: Documentation ✅ ### README Update Added comprehensive **"Telemetry Configuration"** section: - ✅ Opt-in/opt-out examples - ✅ What data IS collected (latency, errors, features) - ✅ What data is NOT collected (SQL, PII, credentials) - ✅ Performance impact (< 1%) - ✅ Links to detailed documentation ### Troubleshooting Guide (`TROUBLESHOOTING.md` - 521 lines) **Common Issues Covered:** - Telemetry not working (diagnostic steps, solutions) - High memory usage (batch size, flush interval tuning) - Performance degradation (overhead measurement, optimization) - Circuit breaker always open (connectivity, error rate checks) - Rate limited errors (per-host sharing verification) - Resource leaks (goroutine monitoring, cleanup verification) **Diagnostic Tools:** - Configuration check commands - Force enable/disable for testing - Circuit breaker state inference - Benchmark and integration test commands **Performance Tuning:** - Reduce overhead (disable, increase intervals) - Optimize for high-throughput (batch size tuning) **Privacy Verification:** - What data is collected - How to verify no sensitive data - Complete opt-out instructions **Support Resources:** - Self-service troubleshooting - Internal support (Slack, JIRA, email) - External support (portal, GitHub issues) - Emergency disable procedures ### Design Documentation Update **DESIGN.md:** - ✅ Marked Phase 8 as completed (all testing items) - ✅ Marked Phase 9 as completed (all launch prep items) - ✅ Marked Phase 10 as completed (all documentation items) --- ## Complete Implementation Status ### All 10 Phases Complete ✅ | Phase | Status | Description | |-------|--------|-------------| | 1 | ✅ | Core Infrastructure | | 2 | ✅ | Per-Host Management | | 3 | ✅ | Circuit Breaker | | 4 | ✅ | Export Infrastructure | | 5 | ✅ | Opt-In Configuration | | 6 | ✅ | Collection & Aggregation | | 7 | ✅ | Driver Integration | | 8 | ✅ | Testing & Validation | | 9 | ✅ | Launch Preparation | | 10 | ✅ | Documentation | --- ## Changes Summary **New Files:** - `telemetry/benchmark_test.go` (392 lines) - `telemetry/integration_test.go` (356 lines) - `telemetry/LAUNCH.md` (360 lines) - `telemetry/TROUBLESHOOTING.md` (521 lines) **Updated Files:** - `README.md` (+40 lines) - `telemetry/DESIGN.md` (marked phases 8-10 complete) **Total:** +1,426 insertions, -40 deletions --- ## Testing **All tests passing:** - ✅ 99 unit tests (existing) - ✅ 6 integration tests (new) - ✅ 6 benchmark tests (new) - ✅ 10 load tests (new) **Total:** 121 tests passing **Benchmark Results:** ``` BenchmarkInterceptor_Overhead_Enabled 36μs/op (< 0.1% overhead) BenchmarkInterceptor_Overhead_Disabled 3.8ns/op (negligible) ``` --- ## Production Ready ✅ The telemetry system is now **complete and production-ready**: - ✅ Comprehensive testing (unit, integration, benchmarks, load) - ✅ Performance validated (< 1% overhead) - ✅ Privacy compliant (no PII, no SQL queries) - ✅ Resilient (circuit breaker, retry, error swallowing) - ✅ Scalable (per-host clients, reference counting) - ✅ Documented (user guide, troubleshooting, launch plan) - ✅ Monitorable (metrics, alerts, dashboards) Ready for phased rollout per LAUNCH.md! --- ## Related Issues - Builds on: #321 (PECOBLR-1383) - Implements: PECOBLR-1384 (Phases 8-10) ✅ - Completes: **Full Telemetry Implementation** 🎉 --- ## Checklist - [x] Phase 8: Testing & Validation - [x] Benchmark tests (overhead < 1%) - [x] Integration tests (all scenarios) - [x] Load tests (100+ connections) - [x] Privacy compliance tests - [x] Phase 9: Launch Preparation - [x] Phased rollout strategy - [x] Monitoring and alerting plan - [x] Rollback procedures - [x] Phase 10: Documentation - [x] README updates - [x] Troubleshooting guide - [x] Launch documentation - [x] All tests passing - [x] Performance validated - [x] Production ready --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
## Summary - Add `.github/actions/setup-jfrog` composite action for OIDC-based JFrog authentication (configures GOPROXY and `.netrc` for Go module proxy) - Switch all workflow jobs (`lint`, `build-and-test`, `dco-check`) from `ubuntu-latest` to `databricks-protected-runner-group` - Add `id-token: write` permission for JFrog OIDC token exchange ## Test plan - [ ] DCO check workflow passes on this PR - [ ] Lint job passes with Go modules resolved through JFrog proxy - [ ] Build and test job passes with Go modules resolved through JFrog proxy - [ ] Verify JFrog OIDC token exchange works on protected runners This pull request was AI-assisted by Isaac. Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 5.6.0 to 6.4.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/actions/setup-go/releases">actions/setup-go's releases</a>.</em></p> <blockquote> <h2>v6.4.0</h2> <h2>What's Changed</h2> <h3>Enhancement</h3> <ul> <li>Add go-download-base-url input for custom Go distributions by <a href="https://github.com/gdams"><code>@gdams</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/721">actions/setup-go#721</a></li> </ul> <h3>Dependency update</h3> <ul> <li>Upgrade minimatch from 3.1.2 to 3.1.5 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/727">actions/setup-go#727</a></li> </ul> <h3>Documentation update</h3> <ul> <li>Rearrange README.md, add advanced-usage.md by <a href="https://github.com/priyagupta108"><code>@priyagupta108</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/724">actions/setup-go#724</a></li> <li>Fix Microsoft build of Go link by <a href="https://github.com/gdams"><code>@gdams</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/734">actions/setup-go#734</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/gdams"><code>@gdams</code></a> made their first contribution in <a href="https://redirect.github.com/actions/setup-go/pull/721">actions/setup-go#721</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/setup-go/compare/v6...v6.4.0">https://github.com/actions/setup-go/compare/v6...v6.4.0</a></p> <h2>v6.3.0</h2> <h2>What's Changed</h2> <ul> <li>Update default Go module caching to use go.mod by <a href="https://github.com/priyagupta108"><code>@priyagupta108</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/705">actions/setup-go#705</a></li> <li>Fix golang download url to go.dev by <a href="https://github.com/178inaba"><code>@178inaba</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/469">actions/setup-go#469</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/setup-go/compare/v6...v6.3.0">https://github.com/actions/setup-go/compare/v6...v6.3.0</a></p> <h2>v6.2.0</h2> <h2>What's Changed</h2> <h3>Enhancements</h3> <ul> <li>Example for restore-only cache in documentation by <a href="https://github.com/aparnajyothi-y"><code>@aparnajyothi-y</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/696">actions/setup-go#696</a></li> <li>Update Node.js version in action.yml by <a href="https://github.com/ccoVeille"><code>@ccoVeille</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/691">actions/setup-go#691</a></li> <li>Documentation update of actions/checkout by <a href="https://github.com/deining"><code>@deining</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/683">actions/setup-go#683</a></li> </ul> <h3>Dependency updates</h3> <ul> <li>Upgrade js-yaml from 3.14.1 to 3.14.2 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/682">actions/setup-go#682</a></li> <li>Upgrade <code>@actions/cache</code> to v5 by <a href="https://github.com/salmanmkc"><code>@salmanmkc</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/695">actions/setup-go#695</a></li> <li>Upgrade actions/checkout from 5 to 6 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/686">actions/setup-go#686</a></li> <li>Upgrade qs from 6.14.0 to 6.14.1 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/703">actions/setup-go#703</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/ccoVeille"><code>@ccoVeille</code></a> made their first contribution in <a href="https://redirect.github.com/actions/setup-go/pull/691">actions/setup-go#691</a></li> <li><a href="https://github.com/deining"><code>@deining</code></a> made their first contribution in <a href="https://redirect.github.com/actions/setup-go/pull/683">actions/setup-go#683</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/setup-go/compare/v6...v6.2.0">https://github.com/actions/setup-go/compare/v6...v6.2.0</a></p> <h2>v6.1.0</h2> <h2>What's Changed</h2> <h3>Enhancements</h3> <ul> <li>Fall back to downloading from go.dev/dl instead of storage.googleapis.com/golang by <a href="https://github.com/nicholasngai"><code>@nicholasngai</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/665">actions/setup-go#665</a></li> <li>Add support for .tool-versions file and update workflow by <a href="https://github.com/priya-kinthali"><code>@priya-kinthali</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/673">actions/setup-go#673</a></li> <li>Add comprehensive breaking changes documentation for v6 by <a href="https://github.com/mahabaleshwars"><code>@mahabaleshwars</code></a> in <a href="https://redirect.github.com/actions/setup-go/pull/674">actions/setup-go#674</a></li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/actions/setup-go/commit/4a3601121dd01d1626a1e23e37211e3254c1c06c"><code>4a36011</code></a> docs: fix Microsoft build of Go link (<a href="https://redirect.github.com/actions/setup-go/issues/734">#734</a>)</li> <li><a href="https://github.com/actions/setup-go/commit/8f19afcc704763637be6b1718da0af52ca05785d"><code>8f19afc</code></a> feat: add go-download-base-url input for custom Go distributions (<a href="https://redirect.github.com/actions/setup-go/issues/721">#721</a>)</li> <li><a href="https://github.com/actions/setup-go/commit/27fdb267c15a8835f1ead03dfa07f89be2bb741a"><code>27fdb26</code></a> Bump minimatch from 3.1.2 to 3.1.5 (<a href="https://redirect.github.com/actions/setup-go/issues/727">#727</a>)</li> <li><a href="https://github.com/actions/setup-go/commit/def8c394e3ad351a79bc93815e4a585520fe993b"><code>def8c39</code></a> Rearrange README.md, add advanced-usage.md (<a href="https://redirect.github.com/actions/setup-go/issues/724">#724</a>)</li> <li><a href="https://github.com/actions/setup-go/commit/4b73464bb391d4059bd26b0524d20df3927bd417"><code>4b73464</code></a> Fix golang download url to go.dev (<a href="https://redirect.github.com/actions/setup-go/issues/469">#469</a>)</li> <li><a href="https://github.com/actions/setup-go/commit/a5f9b05d2d216f63e13859e0d847461041025775"><code>a5f9b05</code></a> Update default Go module caching to use go.mod (<a href="https://redirect.github.com/actions/setup-go/issues/705">#705</a>)</li> <li><a href="https://github.com/actions/setup-go/commit/7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5"><code>7a3fe6c</code></a> Bump qs from 6.14.0 to 6.14.1 (<a href="https://redirect.github.com/actions/setup-go/issues/703">#703</a>)</li> <li><a href="https://github.com/actions/setup-go/commit/b9adafd441833a027479ddd0db37eaece68d35cb"><code>b9adafd</code></a> Bump actions/checkout from 5 to 6 (<a href="https://redirect.github.com/actions/setup-go/issues/686">#686</a>)</li> <li><a href="https://github.com/actions/setup-go/commit/d73f6bcfc2b419b74f47075f8a487b40cc4680f8"><code>d73f6bc</code></a> README.md: correct to actions/checkout@v6 (<a href="https://redirect.github.com/actions/setup-go/issues/683">#683</a>)</li> <li><a href="https://github.com/actions/setup-go/commit/ae252ee6fb24babc50e89fc67c4aa608e69fbf8f"><code>ae252ee</code></a> Bump <code>@actions/cache</code> to v5 (<a href="https://redirect.github.com/actions/setup-go/issues/695">#695</a>)</li> <li>Additional commits viewable in <a href="https://github.com/actions/setup-go/compare/40f1582b2485089dde7abd97c1529aa768e1baff...4a3601121dd01d1626a1e23e37211e3254c1c06c">compare view</a></li> </ul> </details> <br /> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/cache](https://github.com/actions/cache) from 4.3.0 to 5.0.4. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/actions/cache/releases">actions/cache's releases</a>.</em></p> <blockquote> <h2>v5.0.4</h2> <h2>What's Changed</h2> <ul> <li>Add release instructions and update maintainer docs by <a href="https://github.com/Link"><code>@Link</code></a>- in <a href="https://redirect.github.com/actions/cache/pull/1696">actions/cache#1696</a></li> <li>Potential fix for code scanning alert no. 52: Workflow does not contain permissions by <a href="https://github.com/Link"><code>@Link</code></a>- in <a href="https://redirect.github.com/actions/cache/pull/1697">actions/cache#1697</a></li> <li>Fix workflow permissions and cleanup workflow names / formatting by <a href="https://github.com/Link"><code>@Link</code></a>- in <a href="https://redirect.github.com/actions/cache/pull/1699">actions/cache#1699</a></li> <li>docs: Update examples to use the latest version by <a href="https://github.com/XZTDean"><code>@XZTDean</code></a> in <a href="https://redirect.github.com/actions/cache/pull/1690">actions/cache#1690</a></li> <li>Fix proxy integration tests by <a href="https://github.com/Link"><code>@Link</code></a>- in <a href="https://redirect.github.com/actions/cache/pull/1701">actions/cache#1701</a></li> <li>Fix cache key in examples.md for bun.lock by <a href="https://github.com/RyPeck"><code>@RyPeck</code></a> in <a href="https://redirect.github.com/actions/cache/pull/1722">actions/cache#1722</a></li> <li>Update dependencies & patch security vulnerabilities by <a href="https://github.com/Link"><code>@Link</code></a>- in <a href="https://redirect.github.com/actions/cache/pull/1738">actions/cache#1738</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/XZTDean"><code>@XZTDean</code></a> made their first contribution in <a href="https://redirect.github.com/actions/cache/pull/1690">actions/cache#1690</a></li> <li><a href="https://github.com/RyPeck"><code>@RyPeck</code></a> made their first contribution in <a href="https://redirect.github.com/actions/cache/pull/1722">actions/cache#1722</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/cache/compare/v5...v5.0.4">https://github.com/actions/cache/compare/v5...v5.0.4</a></p> <h2>v5.0.3</h2> <h2>What's Changed</h2> <ul> <li>Bump <code>@actions/cache</code> to v5.0.5 (Resolves: <a href="https://github.com/actions/cache/security/dependabot/33">https://github.com/actions/cache/security/dependabot/33</a>)</li> <li>Bump <code>@actions/core</code> to v2.0.3</li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/cache/compare/v5...v5.0.3">https://github.com/actions/cache/compare/v5...v5.0.3</a></p> <h2>v.5.0.2</h2> <h1>v5.0.2</h1> <h2>What's Changed</h2> <p>When creating cache entries, 429s returned from the cache service will not be retried.</p> <h2>v5.0.1</h2> <blockquote> <p>[!IMPORTANT] <strong><code>actions/cache@v5</code> runs on the Node.js 24 runtime and requires a minimum Actions Runner version of <code>2.327.1</code>.</strong></p> <p>If you are using self-hosted runners, ensure they are updated before upgrading.</p> </blockquote> <hr /> <h1>v5.0.1</h1> <h2>What's Changed</h2> <ul> <li>fix: update <code>@actions/cache</code> for Node.js 24 punycode deprecation by <a href="https://github.com/salmanmkc"><code>@salmanmkc</code></a> in <a href="https://redirect.github.com/actions/cache/pull/1685">actions/cache#1685</a></li> <li>prepare release v5.0.1 by <a href="https://github.com/salmanmkc"><code>@salmanmkc</code></a> in <a href="https://redirect.github.com/actions/cache/pull/1686">actions/cache#1686</a></li> </ul> <h1>v5.0.0</h1> <h2>What's Changed</h2> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/actions/cache/blob/main/RELEASES.md">actions/cache's changelog</a>.</em></p> <blockquote> <h1>Releases</h1> <h2>How to prepare a release</h2> <blockquote> <p>[!NOTE]<br /> Relevant for maintainers with write access only.</p> </blockquote> <ol> <li>Switch to a new branch from <code>main</code>.</li> <li>Run <code>npm test</code> to ensure all tests are passing.</li> <li>Update the version in <a href="https://github.com/actions/cache/blob/main/package.json"><code>https://github.com/actions/cache/blob/main/package.json</code></a>.</li> <li>Run <code>npm run build</code> to update the compiled files.</li> <li>Update this <a href="https://github.com/actions/cache/blob/main/RELEASES.md"><code>https://github.com/actions/cache/blob/main/RELEASES.md</code></a> with the new version and changes in the <code>## Changelog</code> section.</li> <li>Run <code>licensed cache</code> to update the license report.</li> <li>Run <code>licensed status</code> and resolve any warnings by updating the <a href="https://github.com/actions/cache/blob/main/.licensed.yml"><code>https://github.com/actions/cache/blob/main/.licensed.yml</code></a> file with the exceptions.</li> <li>Commit your changes and push your branch upstream.</li> <li>Open a pull request against <code>main</code> and get it reviewed and merged.</li> <li>Draft a new release <a href="https://github.com/actions/cache/releases">https://github.com/actions/cache/releases</a> use the same version number used in <code>package.json</code> <ol> <li>Create a new tag with the version number.</li> <li>Auto generate release notes and update them to match the changes you made in <code>RELEASES.md</code>.</li> <li>Toggle the set as the latest release option.</li> <li>Publish the release.</li> </ol> </li> <li>Navigate to <a href="https://github.com/actions/cache/actions/workflows/release-new-action-version.yml">https://github.com/actions/cache/actions/workflows/release-new-action-version.yml</a> <ol> <li>There should be a workflow run queued with the same version number.</li> <li>Approve the run to publish the new version and update the major tags for this action.</li> </ol> </li> </ol> <h2>Changelog</h2> <h3>5.0.4</h3> <ul> <li>Bump <code>minimatch</code> to v3.1.5 (fixes ReDoS via globstar patterns)</li> <li>Bump <code>undici</code> to v6.24.1 (WebSocket decompression bomb protection, header validation fixes)</li> <li>Bump <code>fast-xml-parser</code> to v5.5.6</li> </ul> <h3>5.0.3</h3> <ul> <li>Bump <code>@actions/cache</code> to v5.0.5 (Resolves: <a href="https://github.com/actions/cache/security/dependabot/33">https://github.com/actions/cache/security/dependabot/33</a>)</li> <li>Bump <code>@actions/core</code> to v2.0.3</li> </ul> <h3>5.0.2</h3> <ul> <li>Bump <code>@actions/cache</code> to v5.0.3 <a href="https://redirect.github.com/actions/cache/pull/1692">#1692</a></li> </ul> <h3>5.0.1</h3> <ul> <li>Update <code>@azure/storage-blob</code> to <code>^12.29.1</code> via <code>@actions/cache@5.0.1</code> <a href="https://redirect.github.com/actions/cache/pull/1685">#1685</a></li> </ul> <h3>5.0.0</h3> <blockquote> <p>[!IMPORTANT] <code>actions/cache@v5</code> runs on the Node.js 24 runtime and requires a minimum Actions Runner version of <code>2.327.1</code>.</p> </blockquote> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/actions/cache/commit/668228422ae6a00e4ad889ee87cd7109ec5666a7"><code>6682284</code></a> Merge pull request <a href="https://redirect.github.com/actions/cache/issues/1738">#1738</a> from actions/prepare-v5.0.4</li> <li><a href="https://github.com/actions/cache/commit/e34039626f957d3e3e50843d15c1b20547fc90e2"><code>e340396</code></a> Update RELEASES</li> <li><a href="https://github.com/actions/cache/commit/8a671105293e81530f1af99863cdf94550aba1a6"><code>8a67110</code></a> Add licenses</li> <li><a href="https://github.com/actions/cache/commit/1865903e1b0cb750dda9bc5c58be03424cc62830"><code>1865903</code></a> Update dependencies & patch security vulnerabilities</li> <li><a href="https://github.com/actions/cache/commit/565629816435f6c0b50676926c9b05c254113c0c"><code>5656298</code></a> Merge pull request <a href="https://redirect.github.com/actions/cache/issues/1722">#1722</a> from RyPeck/patch-1</li> <li><a href="https://github.com/actions/cache/commit/4e380d19e192ace8e86f23f32ca6fdec98a673c6"><code>4e380d1</code></a> Fix cache key in examples.md for bun.lock</li> <li><a href="https://github.com/actions/cache/commit/b7e8d49f17405cc70c1c120101943203c98d3a4b"><code>b7e8d49</code></a> Merge pull request <a href="https://redirect.github.com/actions/cache/issues/1701">#1701</a> from actions/Link-/fix-proxy-integration-tests</li> <li><a href="https://github.com/actions/cache/commit/984a21b1cb176a0936f4edafb42be88978f93ef1"><code>984a21b</code></a> Add traffic sanity check step</li> <li><a href="https://github.com/actions/cache/commit/acf2f1f76affe1ef80eee8e56dfddd3b3e5f0fba"><code>acf2f1f</code></a> Fix resolution</li> <li><a href="https://github.com/actions/cache/commit/95a07c51324af6001b4d6ab8dff29f4dfadc2531"><code>95a07c5</code></a> Add wait for proxy</li> <li>Additional commits viewable in <a href="https://github.com/actions/cache/compare/0057852bfaa89a56745cba8c7296529d2fc39830...668228422ae6a00e4ad889ee87cd7109ec5666a7">compare view</a></li> </ul> </details> <br /> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 4.3.1 to 6.0.2. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/actions/checkout/releases">actions/checkout's releases</a>.</em></p> <blockquote> <h2>v6.0.2</h2> <h2>What's Changed</h2> <ul> <li>Add orchestration_id to git user-agent when ACTIONS_ORCHESTRATION_ID is set by <a href="https://github.com/TingluoHuang"><code>@TingluoHuang</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2355">actions/checkout#2355</a></li> <li>Fix tag handling: preserve annotations and explicit fetch-tags by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2356">actions/checkout#2356</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v6.0.1...v6.0.2">https://github.com/actions/checkout/compare/v6.0.1...v6.0.2</a></p> <h2>v6.0.1</h2> <h2>What's Changed</h2> <ul> <li>Update all references from v5 and v4 to v6 by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2314">actions/checkout#2314</a></li> <li>Add worktree support for persist-credentials includeIf by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2327">actions/checkout#2327</a></li> <li>Clarify v6 README by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2328">actions/checkout#2328</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v6...v6.0.1">https://github.com/actions/checkout/compare/v6...v6.0.1</a></p> <h2>v6.0.0</h2> <h2>What's Changed</h2> <ul> <li>Update README to include Node.js 24 support details and requirements by <a href="https://github.com/salmanmkc"><code>@salmanmkc</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2248">actions/checkout#2248</a></li> <li>Persist creds to a separate file by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2286">actions/checkout#2286</a></li> <li>v6-beta by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2298">actions/checkout#2298</a></li> <li>update readme/changelog for v6 by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2311">actions/checkout#2311</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v5.0.0...v6.0.0">https://github.com/actions/checkout/compare/v5.0.0...v6.0.0</a></p> <h2>v6-beta</h2> <h2>What's Changed</h2> <p>Updated persist-credentials to store the credentials under <code>$RUNNER_TEMP</code> instead of directly in the local git config.</p> <p>This requires a minimum Actions Runner version of <a href="https://github.com/actions/runner/releases/tag/v2.329.0">v2.329.0</a> to access the persisted credentials for <a href="https://docs.github.com/en/actions/tutorials/use-containerized-services/create-a-docker-container-action">Docker container action</a> scenarios.</p> <h2>v5.0.1</h2> <h2>What's Changed</h2> <ul> <li>Port v6 cleanup to v5 by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2301">actions/checkout#2301</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v5...v5.0.1">https://github.com/actions/checkout/compare/v5...v5.0.1</a></p> <h2>v5.0.0</h2> <h2>What's Changed</h2> <ul> <li>Update actions checkout to use node 24 by <a href="https://github.com/salmanmkc"><code>@salmanmkc</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2226">actions/checkout#2226</a></li> <li>Prepare v5.0.0 release by <a href="https://github.com/salmanmkc"><code>@salmanmkc</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2238">actions/checkout#2238</a></li> </ul> <h2>⚠️ Minimum Compatible Runner Version</h2> <p><strong>v2.327.1</strong><br /> <a href="https://github.com/actions/runner/releases/tag/v2.327.1">Release Notes</a></p> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/actions/checkout/blob/main/CHANGELOG.md">actions/checkout's changelog</a>.</em></p> <blockquote> <h1>Changelog</h1> <h2>v6.0.2</h2> <ul> <li>Fix tag handling: preserve annotations and explicit fetch-tags by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2356">actions/checkout#2356</a></li> </ul> <h2>v6.0.1</h2> <ul> <li>Add worktree support for persist-credentials includeIf by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2327">actions/checkout#2327</a></li> </ul> <h2>v6.0.0</h2> <ul> <li>Persist creds to a separate file by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2286">actions/checkout#2286</a></li> <li>Update README to include Node.js 24 support details and requirements by <a href="https://github.com/salmanmkc"><code>@salmanmkc</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2248">actions/checkout#2248</a></li> </ul> <h2>v5.0.1</h2> <ul> <li>Port v6 cleanup to v5 by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2301">actions/checkout#2301</a></li> </ul> <h2>v5.0.0</h2> <ul> <li>Update actions checkout to use node 24 by <a href="https://github.com/salmanmkc"><code>@salmanmkc</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2226">actions/checkout#2226</a></li> </ul> <h2>v4.3.1</h2> <ul> <li>Port v6 cleanup to v4 by <a href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2305">actions/checkout#2305</a></li> </ul> <h2>v4.3.0</h2> <ul> <li>docs: update README.md by <a href="https://github.com/motss"><code>@motss</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1971">actions/checkout#1971</a></li> <li>Add internal repos for checking out multiple repositories by <a href="https://github.com/mouismail"><code>@mouismail</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1977">actions/checkout#1977</a></li> <li>Documentation update - add recommended permissions to Readme by <a href="https://github.com/benwells"><code>@benwells</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2043">actions/checkout#2043</a></li> <li>Adjust positioning of user email note and permissions heading by <a href="https://github.com/joshmgross"><code>@joshmgross</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2044">actions/checkout#2044</a></li> <li>Update README.md by <a href="https://github.com/nebuk89"><code>@nebuk89</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2194">actions/checkout#2194</a></li> <li>Update CODEOWNERS for actions by <a href="https://github.com/TingluoHuang"><code>@TingluoHuang</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2224">actions/checkout#2224</a></li> <li>Update package dependencies by <a href="https://github.com/salmanmkc"><code>@salmanmkc</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/2236">actions/checkout#2236</a></li> </ul> <h2>v4.2.2</h2> <ul> <li><code>url-helper.ts</code> now leverages well-known environment variables by <a href="https://github.com/jww3"><code>@jww3</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1941">actions/checkout#1941</a></li> <li>Expand unit test coverage for <code>isGhes</code> by <a href="https://github.com/jww3"><code>@jww3</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1946">actions/checkout#1946</a></li> </ul> <h2>v4.2.1</h2> <ul> <li>Check out other refs/* by commit if provided, fall back to ref by <a href="https://github.com/orhantoy"><code>@orhantoy</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1924">actions/checkout#1924</a></li> </ul> <h2>v4.2.0</h2> <ul> <li>Add Ref and Commit outputs by <a href="https://github.com/lucacome"><code>@lucacome</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1180">actions/checkout#1180</a></li> <li>Dependency updates by <a href="https://github.com/dependabot"><code>@dependabot</code></a>- <a href="https://redirect.github.com/actions/checkout/pull/1777">actions/checkout#1777</a>, <a href="https://redirect.github.com/actions/checkout/pull/1872">actions/checkout#1872</a></li> </ul> <h2>v4.1.7</h2> <ul> <li>Bump the minor-npm-dependencies group across 1 directory with 4 updates by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1739">actions/checkout#1739</a></li> <li>Bump actions/checkout from 3 to 4 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1697">actions/checkout#1697</a></li> <li>Check out other refs/* by commit by <a href="https://github.com/orhantoy"><code>@orhantoy</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1774">actions/checkout#1774</a></li> <li>Pin actions/checkout's own workflows to a known, good, stable version. by <a href="https://github.com/jww3"><code>@jww3</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1776">actions/checkout#1776</a></li> </ul> <h2>v4.1.6</h2> <ul> <li>Check platform to set archive extension appropriately by <a href="https://github.com/cory-miller"><code>@cory-miller</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1732">actions/checkout#1732</a></li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/actions/checkout/commit/de0fac2e4500dabe0009e67214ff5f5447ce83dd"><code>de0fac2</code></a> Fix tag handling: preserve annotations and explicit fetch-tags (<a href="https://redirect.github.com/actions/checkout/issues/2356">#2356</a>)</li> <li><a href="https://github.com/actions/checkout/commit/064fe7f3312418007dea2b49a19844a9ee378f49"><code>064fe7f</code></a> Add orchestration_id to git user-agent when ACTIONS_ORCHESTRATION_ID is set (...</li> <li><a href="https://github.com/actions/checkout/commit/8e8c483db84b4bee98b60c0593521ed34d9990e8"><code>8e8c483</code></a> Clarify v6 README (<a href="https://redirect.github.com/actions/checkout/issues/2328">#2328</a>)</li> <li><a href="https://github.com/actions/checkout/commit/033fa0dc0b82693d8986f1016a0ec2c5e7d9cbb1"><code>033fa0d</code></a> Add worktree support for persist-credentials includeIf (<a href="https://redirect.github.com/actions/checkout/issues/2327">#2327</a>)</li> <li><a href="https://github.com/actions/checkout/commit/c2d88d3ecc89a9ef08eebf45d9637801dcee7eb5"><code>c2d88d3</code></a> Update all references from v5 and v4 to v6 (<a href="https://redirect.github.com/actions/checkout/issues/2314">#2314</a>)</li> <li><a href="https://github.com/actions/checkout/commit/1af3b93b6815bc44a9784bd300feb67ff0d1eeb3"><code>1af3b93</code></a> update readme/changelog for v6 (<a href="https://redirect.github.com/actions/checkout/issues/2311">#2311</a>)</li> <li><a href="https://github.com/actions/checkout/commit/71cf2267d89c5cb81562390fa70a37fa40b1305e"><code>71cf226</code></a> v6-beta (<a href="https://redirect.github.com/actions/checkout/issues/2298">#2298</a>)</li> <li><a href="https://github.com/actions/checkout/commit/069c6959146423d11cd0184e6accf28f9d45f06e"><code>069c695</code></a> Persist creds to a separate file (<a href="https://redirect.github.com/actions/checkout/issues/2286">#2286</a>)</li> <li><a href="https://github.com/actions/checkout/commit/ff7abcd0c3c05ccf6adc123a8cd1fd4fb30fb493"><code>ff7abcd</code></a> Update README to include Node.js 24 support details and requirements (<a href="https://redirect.github.com/actions/checkout/issues/2248">#2248</a>)</li> <li><a href="https://github.com/actions/checkout/commit/08c6903cd8c0fde910a37f88322edcfb5dd907a8"><code>08c6903</code></a> Prepare v5.0.0 release (<a href="https://redirect.github.com/actions/checkout/issues/2238">#2238</a>)</li> <li>Additional commits viewable in <a href="https://github.com/actions/checkout/compare/34e114876b0b11c390a56381ad16ebd13914f8d5...de0fac2e4500dabe0009e67214ff5f5447ce83dd">compare view</a></li> </ul> </details> <br /> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…nt correctness tests (#349) ## Summary This PR extends the telemetry implementation across two areas. ### 1. DSN / config changes (original scope) - `EnableTelemetry *bool` tristate in `telemetry.Config`: `nil` = defer to server flag, `&true` = client opt-in, `&false` = client opt-out. - Two-level enable priority: DSN `enableTelemetry=true` → always on; otherwise use server feature flag. - Two new DSN params: `telemetry_retry_count` and `telemetry_retry_delay`. ### 2. Telemetry gap fixes (new in this PR) Four correctness bugs found during end-to-end testing against a real warehouse: **EXECUTE_STATEMENT / CLOSE_STATEMENT silently lost on shutdown** Root cause: `agg.cancel()` fired while a worker was mid-HTTP-export. Fix: added `inFlight sync.WaitGroup`; `close()` calls `inFlight.Wait()` before `cancel()`. **`total_chunks_present: null` for paginated CloudFetch** Root cause: server reports 1 link per `FetchResults` call; grand total never in a single response. Fix: pass `r.chunkCount` through `closeCallback`; `connection.go` sets `chunk_total_present` if the server never reported it. **`operation_latency_ms: null` for CLOSE_STATEMENT** Root cause: `CloseOperation` RPC completes in <1ms → rounds to 0; `omitempty` drops 0. Fix: removed `omitempty` from `OperationLatencyMs`. **CloudFetch S3 timing fields not populated** Root cause: per-S3-file download time was not measured. Fix: added `onFileDownloaded func(downloadMs int64)` callback to `cloudIPCStreamIterator`; `connection.go` aggregates initial/slowest/sum timings. ### 3. DSN parameters (full set) | Parameter | Type | Default | Description | |---|---|---|---| | `enableTelemetry` | bool | unset | Overrides server flag when set | | `telemetry_batch_size` | int | 100 | Events per batch | | `telemetry_flush_interval` | duration | 5s | Periodic flush interval | | `telemetry_retry_count` | int | 3 | Max retry attempts on export failure | | `telemetry_retry_delay` | duration | 100ms | Base delay between retries (exponential backoff) | ## Key files changed - `telemetry/aggregator.go` — `inFlight` WaitGroup; 5-step `close()` ordering - `telemetry/interceptor.go` — `RecordOperation` takes `statementID` so CLOSE_STATEMENT carries `sql_statement_id` - `telemetry/request.go` — removed `omitempty` from `OperationLatencyMs` - `connection.go` — `closeCallback(latencyMs, chunkCount, err)` + `cloudFetchCallback` wiring - `internal/rows/rows.go` — `closeCallback` passes `r.chunkCount`; `cloudFetchCallback` threaded through - `internal/rows/arrowbased/batchloader.go` — `onFileDownloaded` callback per S3 file download ## Test plan - [x] `go build ./...` — clean compile - [x] `go test ./telemetry/... -count=1` — all pass - [x] `go test ./internal/rows/... -count=1` — all pass - [x] `go test ./... -short -count=1` — full suite passes ### New correctness tests **`telemetry/aggregator_test.go`** (new file, 5 tests): - `WaitsForInFlightWorkerExports` — `close()` blocks until all HTTP exports finish, even if workers picked up jobs before the drain step ran - `DrainsPendingQueueJobsBeforeCancel` — jobs sitting in `exportQueue` are exported synchronously during drain - `InFlightAddBeforeSend` — `inFlight.Add(1)` precedes the channel send so no job is invisible to `Wait()` - `SafeToCallMultipleTimes` — concurrent `close()` calls do not deadlock (`sync.Once`) - `DropWhenQueueFull` — drop path calls `inFlight.Done()` so `Wait()` is never permanently blocked **`telemetry/integration_test.go`** (2 new tests): - `OperationLatencyMs_ZeroNotOmitted` — raw JSON contains `"operation_latency_ms":0`, not absent - `ChunkTotalPresent_DerivedFromChunkCount` — `chunk_total_present` tag propagates to `ChunkDetails` **`internal/rows/arrowbased/batchloader_test.go`** (2 new tests): - `OnFileDownloaded` callback invoked once per file with positive `downloadMs` - Nil callback is safe on non-telemetry paths (no panic) **`internal/rows/rows_test.go`** (2 new tests): - `CloseCallback_ReceivesChunkCount` — callback gets correct total pages after multi-page iteration - `CloseCallback_NilDoesNotPanic` — nil `closeCallback` is safe on `rows.Close()` This pull request was AI-assisted by Isaac.
## Summary - Adds a new optional bool field `enforceEmbeddedSchemaCorrectness` (field ID `0xD19` / 3353) to `TExecuteStatementReq` in the thrift contract - Exposes it as an opt-in configuration parameter (`EnforceEmbeddedSchemaCorrectness`, default `false`) via connector option, DSN parameter, and config struct - When enabled (`true`), the field is set in the thrift request so the server enforces embedded schema correctness during query execution ## Changes - `internal/cli_service/cli_service.go` — Added field with full Read/Write/IsSet/Equals support following existing `*bool` pointer pattern - `internal/config/config.go` — Added `EnforceEmbeddedSchemaCorrectness` to `UserConfig` + DSN parsing - `connector.go` — Added `WithEnforceEmbeddedSchemaCorrectness()` connector option - `connection.go` — Wired config to `TExecuteStatementReq` in `executeStatement()` ## Test plan - [ ] Verify compilation passes - [ ] Test with `enforceEmbeddedSchemaCorrectness=true` in DSN - [ ] Test with `WithEnforceEmbeddedSchemaCorrectness(true)` connector option - [ ] Verify field is serialized in thrift request when enabled - [ ] Verify field is not sent when disabled (default) Resolves: ES-1804970 This pull request was AI-assisted by Isaac.
…344)" (#350) ## Summary - Reverts #344 (commit 3f115aa), which added the `enforceEmbeddedSchemaCorrectness` field to `TExecuteStatementReq` along with the DSN/connector/config plumbing. ## Test plan - [ ] CI passes on the revert - [ ] Confirm no remaining references to `EnforceEmbeddedSchemaCorrectness` in the codebase This pull request was AI-assisted by Isaac.
…results (#351) ## Summary Fixes a bug where `arrow.Record.Schema()` returns stale column aliases when CloudFetch serves cached Arrow IPC files from a structurally identical prior query with different `AS` aliases. - **Root cause:** `NewCloudBatchIterator` was not receiving the authoritative schema bytes from `GetResultSetMetadata`, unlike the local batch path which already had this. CloudFetch Arrow IPC files have column names baked in from the original query, and the driver was reading them as-is. - **Fix:** Pass `arrowSchemaBytes` (the authoritative schema from `GetResultSetMetadata`) into `NewCloudBatchIterator`. After records are deserialized from the IPC stream, replace the stale schema with the authoritative one using `array.NewRecord()` (zero-copy — shares underlying column data, only swaps metadata). ## Changes - **`arrowRecordIterator.go`** — Pass `ri.arrowSchemaBytes` to `NewCloudBatchIterator` in `newBatchIterator()` - **`arrowRows.go`** — Pass `schemaBytes` to `NewCloudBatchIterator` in `NewArrowRowScanner()` - **`batchloader.go`** — Core fix: - `NewCloudBatchIterator` accepts `arrowSchemaBytes`, parses into `*arrow.Schema`, stores on `batchIterator` - `batchIterator.Next()` applies override schema to CloudFetch records only (local path is untouched, `overrideSchema` is `nil`) - Added `schemaFromIPCBytes()` helper - Field count validation guard to prevent panics on schema mismatch - Schema parse failure logged at `Warn` level - **`batchloader_test.go`** — Added `TestCloudFetchSchemaOverride` with two subtests: - Verifies stale column names `["id","name"]` are overridden to `["x","y"]` - Verifies `nil` schema bytes pass through original names unchanged ## Who is affected Go driver users with CloudFetch enabled (`WithCloudFetch(true)`) who read `arrow.Record.Schema()` directly. Python, ODBC, and JDBC drivers are not affected. ## Test plan - [x] All existing unit tests pass (37 tests in `internal/rows/arrowbased/`) - [x] New unit test `TestCloudFetchSchemaOverride` covers the override and no-override paths - [x] Verified end-to-end against a real Databricks warehouse using `samples.tpch.lineitem` (~30M rows) with two queries differing only in column aliases — confirmed `arrow.Record.Schema()` now returns correct aliases This pull request was AI-assisted by Isaac. --------- Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
…347) ## Summary On SPOG (Custom URL / account-level) workspaces, `httpPath` has the form `/sql/1.0/warehouses/<id>?o=<workspaceId>`. The `?o=` parameter routes Thrift calls correctly via the URL, but other endpoints (telemetry push, feature-flag check) run on separate hosts and need `x-databricks-org-id` as an HTTP header to route to the right workspace. Without it, those requests 404 or get misrouted on SPOG hosts. ## Change All contained in `connector.go`: 1. `extractSpogHeaders(httpPath string) map[string]string` — parses the `?o=` query param using `url.ParseQuery` (stdlib, not regex). Returns `{"x-databricks-org-id": "<workspaceId>"}` or `nil`. Three DEBUG log paths cover: malformed query, missing `?o=`, and successful extraction. 2. `headerInjectingTransport` — a lightweight `http.RoundTripper` wrapper that clones the request per the contract and sets the provided headers if not already set by the caller. 3. `withSpogHeaders(base *http.Client, headers map[string]string) *http.Client` — returns a new client with the same settings but a wrapped transport. 4. In `Connect()`, when `extractSpogHeaders` returns non-nil, the driver passes a wrapped client into `TelemetryInitOptions.HTTPClient`. The wrapped client is used for both the feature-flag check and the telemetry push. The driver's own `c.client` is left alone, so Thrift routing (which uses `?o=` in the URL) is unaffected. ## Why a transport wrapper instead of threading a parameter An earlier revision of this PR threaded an `extraHeaders` parameter through `telemetry.TelemetryInitOptions` → `isTelemetryEnabled` → `featureFlagCache.isTelemetryEnabled` → `fetchFeatureFlag`. That approach: - Required API-surface changes in 3 telemetry files (`config.go`, `featureflag.go`, `driver_integration.go`). - Only covered the feature-flag check; `telemetry/exporter.go` (telemetry push) still sent `Content-Type` as the only header — SPOG routing would 404 at push time. The RoundTripper wrapper: - Keeps `telemetry/*` identical to `origin/main`. Zero API churn. - Automatically applies to every outbound request using the wrapped client — feature-flag check, telemetry push, and any future HTTP paths that reuse it. - Respects caller-set headers (if a request already has `x-databricks-org-id` for some reason, the wrapper does not override). ## Endpoints covered | Endpoint | Uses wrapped client? | Gets `x-databricks-org-id`? | | :---- | :----: | :----: | | Feature flags `/api/2.0/connector-service/feature-flags/GOLANG/{v}` | Yes | ✅ | | Telemetry push | Yes | ✅ | | Thrift | No (uses `c.client` directly; already routes via URL) | — (not needed — URL-routed) | | OAuth token exchange | No (separate client, talks to `login.microsoftonline.com`) | — (not needed) | | CloudFetch / Volume operations | No (presigned URLs) | — (not needed) | ## Verification - `go build ./...` clean. - Debug log output confirms extraction on SPOG URLs: ``` SPOG header extraction: injecting x-databricks-org-id=<id> (extracted from ?o= in httpPath) ``` ## Note on the earlier `auth/oauth/u2m/u2m.go` change The two earliest commits on this branch (`23697e5`, `0ec7e06`) modified `auth/oauth/u2m/u2m.go` to avoid sending an empty `client_secret` on the PKCE public-app flow, documented as a fix for the server's `"Public app should not use a client secret"` rejection. **That change was empirically verified to not be needed** (see commit `3576c92` which reverts it): - **Prod Legacy** (`adb-6436897454825492.12.azuredatabricks.net`): Unpatched `u2m.go` PASSES end-to-end. Server accepts the empty `client_secret`. - **Stg Legacy** (`adb-7064161269814046.2.staging.azuredatabricks.net`): Unpatched `u2m.go` FAILS with `unexpected HTTP status 400 Bad Request` during token exchange. Since production tolerates the current behavior, customers aren't impacted. Keeping this PR minimal; if staging-level strictness later rolls out to prod, we can re-add the u2m fix then. ## Test plan - [ ] Unit tests pass (`go test ./...`) - [ ] Manually verified SPOG header injection appears in `x-databricks-org-id` on feature-flag check and telemetry push against a SPOG workspace (not gated by this PR, but prerequisite for SPOG telemetry to work at all) - [ ] No regressions on non-SPOG workspaces (wrapper is a no-op when `extractSpogHeaders` returns nil) --------- Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com> Signed-off-by: Madhavendra Rathore Co-authored-by: Samikshya Chand <148681192+samikshya-db@users.noreply.github.com>
## Summary Bump `DriverVersion` to `1.11.0` and add the v1.11.0 section to `CHANGELOG.md`. ### Changes since v1.10.0 - Enable telemetry by default with DSN-controlled priority (#320, #321, #322, #349) - Add SPOG (Custom URL) routing support via `x-databricks-org-id` header (#347) - Add statement-level query tag support (#341) - Add AI coding agent detection to User-Agent header (#326) - Fix CloudFetch returning stale column names from cached results (#351) - Fix resource leak: close staging Rows in execStagingOperation (#325) Internal/infra-only changes are omitted from the user-facing notes (CI hardening, dependabot bumps, CODEOWNERS). ## Test plan - [x] `go build ./...` clean - [x] `go test ./... -count=1 -short` passes locally ## Next steps after merge 1. Tag the merge commit as `v1.11.0` and push the tag 2. Trigger `peco-databricks-sql-go` in secure-public-registry-releases-eng with `ref=v1.11.0`, `dry-run=true` to verify 3. Re-run with `dry-run=false` for the actual release NO_CHANGELOG=true This pull request was AI-assisted by Isaac. Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…357) ## Summary Fixes #356. Under high CloudFetch concurrency (≥6 simultaneous downloads), in-flight `cloudFetchDownloadTask` goroutines could leak when the consumer closed the iterator before draining all results. Each leaked goroutine pinned a downloaded chunk in the Go heap, producing the multi-GiB heap plateau described in the issue that only released on process restart. ## Root cause `cloudFetchDownloadTask.Run` sends the download result on an **unbuffered** channel without honoring context cancellation: ```go cft.resultChan <- cloudFetchDownloadTaskResult{data: bytes.NewReader(buf), ...} ``` Sequence that triggers the leak: 1. `cloudIPCStreamIterator.Next` schedules `MaxDownloadThreads` (default 10) tasks concurrently. 2. The consumer dequeues task 1, gets its result, returns. 3. Tasks 2..N have completed their HTTP read in parallel and are now **blocked** on the unbuffered send, holding their downloaded buffer. 4. The consumer abandons the iterator (timeout, error, early close, etc.) and calls `iterator.Close()`. 5. `Close` calls `task.cancel()` on each remaining task. But context cancellation does **not** unblock an in-flight channel send — the goroutines stay blocked forever, retaining their buffers. In v1.7.1 (the version the reporter is on) the goroutine had already decoded the bytes into Arrow records *before* the send, so the leaked memory was Arrow-allocator buffers — matching the stack trace in the issue: ``` (*cloudFetchDownloadTask).Run.func1 getArrowRecords → (*ipc.Reader).Next → newRecord → loadArray → loadBinary → buffer → (*ipcSource).buffer → NewResizableBuffer → (*Buffer).Resize → (*GoAllocator).Allocate ``` In the current code (v1.11.0) the decode happens later in `batchIterator.Next`, so the leak is the raw decompressed `buf` instead — same shape, smaller per-goroutine retention, same plateau pattern. ## Fix Route every channel send through a helper that selects on `ctx.Done()`: ```go func (cft *cloudFetchDownloadTask) sendResult(result cloudFetchDownloadTaskResult) { select { case cft.resultChan <- result: case <-cft.ctx.Done(): } } ``` `cloudIPCStreamIterator.Close` already calls `task.cancel()` for every queued task, so cancellation now correctly drains stuck goroutines and lets their buffers be GC'd. ## Test plan - [x] New unit test `TestCloudFetchIterator_CloseReleasesInFlightDownloads` reproduces the leak: spawns `MaxDownloadThreads` concurrent downloads, releases them after the iterator has consumed only the first, then calls `Close()` and asserts that no `cloudFetchDownloadTask.Run` goroutines remain. - Fails on `main` (~9 leaked goroutines after `Close`). - Passes with this change. - [x] Full `go test ./...` passes locally. - [x] `go vet` and `gofmt` clean. ## Who is affected Any user with CloudFetch enabled (default since v1.7.0) whose query context can be cancelled or whose result set can be abandoned mid-stream — i.e., basically everyone running large CloudFetch queries with timeouts. This pull request and its description were written by Isaac. Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
## Summary Bump `DriverVersion` to `1.11.1` and add the `v1.11.1` section to `CHANGELOG.md`. ### Notable changes since v1.11.0 - Fix CloudFetch goroutine leak that retained Arrow buffers after Close (#357) ## Test plan - [x] `go test ./... -count=1 -short` passes locally - [x] Multi-DBR tests in `universe/peco/correctness/go` passed (5/5 suites on DBR 18.x-photon-scala2.13) - [ ] CI green This pull request was AI-assisted by Isaac. Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
## Summary Addresses [SIRT-1753](https://databricks.atlassian.net/browse/SIRT-1753) by bumping the transitive `github.com/go-jose/go-jose/v3` dependency from `v3.0.4` to `v3.0.5`, which patches **CVE-2026-34986** (JWE `KeyUnwrap` panic → DoS). - `go-jose` is an indirect dependency pulled in via `coreos/go-oidc/v3`. - The vulnerable code path (`ParseEncrypted*` → `Decrypt`) is never reached by this driver — `go-oidc` is used only for OIDC provider discovery (fetching `.well-known/openid-configuration`). - The bump is to satisfy SCA scanners; it is not a functional change. ## Scope note: CVE-2026-41602 (apache/thrift) is intentionally **not** included SIRT-1753 also flags `github.com/apache/thrift v0.17.0` for **CVE-2026-41602** (TFramedTransport integer overflow). This PR does **not** bump thrift, for two reasons: 1. **The upstream fix only lands in `apache/thrift v0.23.0`**, which requires **Go 1.25**. This module's `go.mod` is pinned to `go 1.20`. Bumping the go directive to 1.25 would force every downstream consumer on Go 1.20–1.24 to upgrade their build toolchain — a breaking change inappropriate for a security patch release. 2. **The vulnerable component is not reachable.** Per [Ricardo's analysis on SIRT-1753](https://databricks.atlassian.net/browse/SIRT-1753), the driver only uses `THttpClient` (see `internal/client/client.go:283` and the hardcoded `ThriftTransport: "http"` default in `internal/config/config.go`). `TFramedTransport` is never instantiated — there is no code path, even a fallback, that constructs it. The non-exploitability of CVE-2026-41602 will be communicated back to SIRT (VEX / suppression) rather than addressed via a toolchain bump. We can revisit when the driver's Go floor moves as part of a planned, communicated minor release. ## Test plan - [x] `go build ./...` — clean - [x] `go test ./...` — all packages pass This pull request and its description were written by Isaac. [SIRT-1753]: https://databricks.atlassian.net/browse/SIRT-1753?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
fetchBatchBytes had no retry on HTTP failures, so any single 5xx from S3 cancelled the entire query. With thousands of concurrent GETs on large result sets, even a sub-percent per-request failure rate makes at least one failure near-certain. Adds exponential backoff with equal jitter for 408/429/500/502/503/504 plus connection errors, honoring the existing RetryMax / RetryWaitMin / RetryWaitMax config and parseable integer Retry-After response headers. Link expiry is re-checked after each backoff so retries don't outlive the presigned URL. Co-authored-by: Isaac --------- Signed-off-by: Jayant Singh <jayant.singh@databricks.com>
## Summary Three dependency bumps surfaced by OSV-Scanner against `go.mod`. **All stay within the existing `go 1.20` directive** — each fixed version declares `go 1.17`/`1.18` in its own `go.mod`, so no Go-toolchain change is forced by this PR. | Dependency | From | To | Severity | CVE | |---|---|---|---|---| | `golang-jwt/jwt/v5` | 5.2.1 | **5.2.2** | HIGH 8.7 | GO-2025-3553 / GHSA-mh63-6h87-95cp | | `google.golang.org/protobuf` | 1.28.1 | **1.33.0** | HIGH 7.5 | GO-2024-2611 / GHSA-8r3f-844c-mc37 | | `golang.org/x/net` | 0.21.0 | **0.33.0** | MED 5.3 + LOW | GO-2024-2687 + GO-2024-3333 | ### Why these specific patch versions - **`jwt/v5.2.2`** is the backport of the fix; `v5.3.0` forces `go 1.21`. - **`protobuf 1.33.0`** is the lowest patched version; declares `go 1.17`. - **`x/net 0.33.0`** is the highest `x/net` we can take while staying on `go 1.20` — `v0.36.0+` forces `go 1.23`. The remaining 4 `x/net` advisories (`GO-2025-3503`, `GO-2025-3595`, `GO-2026-4440/4441/4918`) will be cleared by a follow-up Go-toolchain bump. ### Net OSV-Scanner result after this PR ``` HIGH: 5 -> 3 (apache/thrift, x/crypto, x/oauth2 remain — all require go >= 1.23) MED: 5 -> 4 LOW: 60 -> 59 (55 of the LOWs are stdlib@1.20.x advisories that only clear when the build toolchain itself is upgraded) ``` ## Test plan - [x] `go build ./...` clean - [x] `go mod tidy` no further changes - [x] OSV-Scanner v2.3.8 confirms the predicted drop - [x] `go 1.20` directive in `go.mod` unchanged - [ ] Existing test suite (CI on this PR exercises this) This pull request was AI-assisted by Isaac. Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
## Summary Sibling of [ES-1892645](https://databricks.atlassian.net/browse/ES-1892645) / PR #355 (the CloudFetch retry fix that just merged). Same FactSet customer, same root cause class (transient S3 5xx), different code path. The three staging-operation HTTP wrappers in `connection.go` (`handleStagingPut`, `handleStagingGet`, `handleStagingRemove`) make a single `client.Do(req)` call with no retry. Under FactSet's load test (~30 PUTs / 2 min against a UC external volume), S3 intermittently returns `503 SlowDown` and the driver fails the entire SQL statement permanently: ``` staging operation over HTTP was unsuccessful: 503-<S3 SlowDown body> ``` ## Changes - **`connection.go`** — adds `doStagingRequestWithRetry`, a per-conn helper that wraps a `func(attempt int) (*http.Request, error)` factory in a retry loop. All three `handleStaging*` methods use it. - **PUT body lifecycle** — `http.Client.Do` consumes the request body (an `*os.File`) on each attempt, so the retry helper `Seek(0, SeekStart)`s the file between attempts. The file is also wrapped in `io.NopCloser` so the client can't close it; the outer `defer dat.Close()` owns the lifecycle. - **`internal/retry/retry.go`** (new) — factors out `RetryableStatuses`, `IsRetryableStatus`, and `Backoff` so the CloudFetch path and the staging path share one implementation. Addresses the "two divergent retry implementations" follow-up from #355. - **`internal/rows/arrowbased/batchloader.go`** — migrated to use the shared `retry` package. Same behavior, no functional change. ## Retry semantics — consistent with PR #355 (the customer asked) | Behavior | This PR (staging) | PR #355 (CloudFetch) | |---|---|---| | Retryable statuses | 408/429/500/502/503/504 | identical (shared via `internal/retry`) | | Backoff curve | Exponential with equal jitter, capped at `RetryWaitMax` | identical | | Integer `Retry-After` honored | yes (capped at `RetryWaitMax`) | identical | | Context cancel during backoff | aborts promptly | identical | | Config knobs | `RetryMax` / `RetryWaitMin` / `RetryWaitMax` | identical | ## Test plan Added `TestConn_handleStagingRetry` in `connection_test.go` with 8 subtests: - [x] PUT retries transient 503 and eventually succeeds - [x] GET retries transient 503 and eventually succeeds - [x] REMOVE retries transient 503 and eventually succeeds - [x] PUT retries transient HTTP 500 - [x] PUT fails after exhausting retries on persistent 503 (verifies attempt count = `RetryMax+1`) - [x] PUT does not retry non-retryable status (403) - [x] **PUT replays the file body on each retry** — server verifies full payload bytes arrive on attempts 1, 2, and 3 (catches the `Seek`/`NopCloser` regression class) - [x] PUT respects context cancellation during backoff Plus moved `TestCloudFetchBackoff` and `TestCloudFetchRetryableStatus` into the new `internal/retry/retry_test.go` package (no behavioral changes, just relocated to live with the shared helpers). ### Verified - [x] All new tests fail on `main` (`origin/main` SHA `a97b104`) — confirmed reproduction of the bug. - [x] All new tests pass on this branch. - [x] Full test suite passes locally: `go test ./... -short`. - [x] `go vet ./...` clean. - [x] `gofmt -l` clean (only pre-existing generated-file diffs in `internal/cli_service/`). ## Related-pattern audit Per the `/fix-github-issue` Step 7, searched for other single-shot `client.Do(req)` sites that might need the same treatment: | Site | Status | |---|---| | `internal/rows/arrowbased/batchloader.go` (CloudFetch) | already retried via #355; now shares helpers | | `telemetry/exporter.go` | already has its own retry loop | | `telemetry/featureflag.go` | feature-flag fetch, not customer data path — out of scope | | `auth/tokenprovider/exchange.go` | token exchange — different concern (auth), file a separate ticket if needed | No other staging-like sites need retrofitting in this PR. ## Customer-facing answers (from JIRA) > Are we going to expect consistent behavior across both the in-flight CloudFetch fix and this more recent PUT/GET/REMOVE use case? **Yes** — same retryable statuses, same backoff curve, same config knobs. The two paths now share one implementation in `internal/retry`. > Will the backoff retry be configurable? **Yes, via the existing `RetryMax` / `RetryWaitMin` / `RetryWaitMax` config knobs.** No new API surface; the staging path opts into the same retry budget the driver already exposes for Thrift and (post-#355) CloudFetch. This pull request and its description were written by Isaac. [ES-1892645]: https://databricks.atlassian.net/browse/ES-1892645?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…#354) ## Summary After `v1.11.0` enabled telemetry by default via the server feature flag, high-QPS workloads produced excessive 429s on `/telemetry-ext`. Three issues compounded: 1. **Double-retry.** The telemetry exporter ran its own retry loop on top of the retryablehttp-wrapped HTTP client (`internal/client.RetryableClient`), which **already** retries 429/5xx with `Retry-After`. Result: up to `RetryMax × (MaxRetries+1)` HTTP attempts per export, all collapsed into a single circuit-breaker outcome — so the breaker barely opened against persistent throttling. 2. **Untraceable in access logs.** Telemetry POSTs and feature-flag GETs sent no `User-Agent`, so 429s landed in access logs tagged as `Go-http-client/1.1` and could not be attributed to `godatabrickssqlconnector` by driver version. 3. **High request volume.** `FlushInterval=5s` / `BatchSize=100`. ## Changes ### Retry behavior - **`telemetry/exporter.go`** — Removed `doExport`'s retry loop entirely. doExport now makes a single HTTP request; transient retries (429/5xx, `Retry-After`) are owned by the underlying retryablehttp client. Each `export()` call now corresponds to exactly one HTTP transaction = one breaker outcome. - **`telemetry/config.go`, `telemetry/driver_integration.go`** — Removed `MaxRetries` / `RetryDelay` from `telemetry.Config` and `TelemetryInitOptions`. `telemetry_retry_count` / `telemetry_retry_delay` DSN params still parse without error for backwards compatibility but are no-ops. ### Identifiability - **`connector.go`** — New `buildUserAgent` helper mirroring `internal/client/client.go:295-302` exactly: `DriverName/DriverVersion` + optional `UserAgentEntry` + agent product. - **`telemetry/exporter.go`, `telemetry/featureflag.go`** — Set `User-Agent` on telemetry POST and feature-flag GET. Plumbed via `TelemetryInitOptions.UserAgent`. ### Cadence and breaker tuning - **`telemetry/config.go`** — `FlushInterval` 5s → 30s, `BatchSize` 100 → 200. - **`telemetry/circuitbreaker.go`** — `minimumNumberOfCalls` 20 → 10 (so low-traffic clients can trip the breaker now that each export is one signal), `waitDurationInOpenState` 30s → 60s (respect typical `Retry-After`). ### Tests - Removed obsolete retry/backoff tests (`TestExport_RetryOn5xx`, `TestExport_ExponentialBackoff`, `TestIsRetryableStatus`, retry-config parsing tests). - Added `TestExport_SingleAttemptPerExport` covering 4xx/429/5xx, asserting the exporter never retries. - Added `TestExport_SetsUserAgent` and `TestFetchFeatureFlag_SetsUserAgent`. ## Mitigation While this rolls out, users can opt out via DSN: `enableTelemetry=false`. Server-side: disable `enableTelemetryForGoDriver` for affected workspaces. ## Test plan - [x] `go test ./...` — all green locally. - [x] Exporter never retries (4xx, 429, 500, 503). - [x] User-Agent set on telemetry POST and feature-flag GET. - [ ] Verify in Lumberjack post-deploy that `/telemetry-ext` and `/api/2.0/connector-service/feature-flags/GOLANG/...` requests carry `godatabrickssqlconnector/<version>` in `http_user_agent`. - [ ] Confirm 429 rate against `/telemetry-ext` drops after rollout. This pull request and its description were written by Isaac.
#364) ## Summary - `clientManager` and `circuitBreakerManager` keyed their maps by the raw host string, so DSN variants like `example.com`, `example.com/`, and `https://example.com` created separate telemetry clients and separate circuit breakers for the same logical host. That fragments trip state and defeats the per-host consolidation the managers are designed to do. - Add `normalizeHostKey` (lowercase, trim whitespace, strip `http(s)://`, trim trailing slashes) and use it for all registry lookups in `clientManager.getOrCreateClient`, `clientManager.releaseClient`, and `circuitBreakerManager.getCircuitBreaker`. - The host string flowing into URL construction is unchanged — first-caller-wins, matching the existing user-agent semantics documented on `getOrCreateClient`. Addresses Gopal's review comment on #354: #354 (comment)... (host normalization on `telemetry/exporter.go:62`). ## Test plan - [x] `TestNormalizeHostKey` — covers scheme stripping, lowercase, trim trailing slash, whitespace, empty string - [x] `TestClientManager_HostVariantsShareClient` — four DSN variants of the same host share a single telemetry client; release via a different variant still cleans up - [x] `TestCircuitBreakerManager_HostVariantsShareBreaker` — same coverage for the breaker registry - [x] Full `go test ./telemetry/...` passes This pull request and its description were written by Isaac.
On a unified/SPOG host the workspace id rides in the HTTP path's ?o=<org>, and the kernel injects x-databricks-org-id only when the path goes through set_http_path (kernel-side ConnectionConfig::from_http_path parses both the warehouse id and ?o=). set_warehouse takes only host+id and drops the org id, so a WithWarehouseID-addressed SPOG session 303s to /login. The kernel also refuses a caller-supplied x-databricks-org-id custom header, so the org id can reach the kernel only via the path. When a warehouse id is set but the HTTP path is a canonical warehouses/endpoints path carrying a non-empty ?o=, route by the path instead: the kernel still parses the same warehouse id out of it, plus the org id it needs. Guarded by httpPathCarriesOrgRouting so non-SPOG connections (no ?o=) keep the existing set_warehouse routing untouched. Go-only: Python and Node always use the http_path route, and the ODBC driver calls set_http_path with the raw path, so all three already parse ?o=. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Onboards both engine bots to databricks-sql-go, PAT-free (App-auth), pinned to engine SHA d05dcb11. Single PR, adapted for the Go/Makefile toolchain. Reviewer bot (read-only): fires on PR open/update + workflow_dispatch; Python interpreter + JFrog + engine install; posts inline findings. No existing Claude automation in this repo, so it's the sole autonomous reviewer — no conflict. Engineer bot (bug-fix flow, live E2E discipline): on a maintainer `engineer-bot` label. Sets up Go 1.25 + JFrog GOPROXY, builds pure-Go (CGO_ENABLED=0 — NOT the opt-in SEA/kernel path), then requires a live driver_e2e_test.go repro against a warehouse over the Thrift backend before fixing. Follow-up runs the pure-Go `make test` unit suite only (no live creds). .bot/: engineer config + prompts for Go; reviewer additive prompt. bash_allowlist is go/make (go build, go test, make test, make lint, gofmt -l, git status) — excludes go get / go mod tidy / make fmt (would dirty go.sum or rewrite files). denied_subpaths blocks .github/, build/ (kernel-lib scripts + native artifacts), testdata/ (fixtures). pecotesting_creds_test.go gains a token fallback so DATABRICKS_PECOTESTING_TOKEN survives the engine's env scrub (the *_TOKEN vars are stripped from the agent subprocess; without it the e2e repro silently t.Skips); env var still wins, so normal CI/local dev is unchanged. Two local composites (bot-prelude, install-bot-engine) mirror the sql-python / odbc / nodejs onboardings — the engine can't be `uses:`-d cross-repo, so it's pip-installed in REF mode. Runs on the protected runner group + JFrog mirror like the other internal drivers. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
Applies the reviewer-bot hardening finding (raised on the odbc onboarding PR) to this repo: - Reviewer + reviewer-followup: drop the redundant `setup-jfrog` step. The engine install (install-bot-engine, use-jfrog=true) already mints + scopes its own JFrog creds in job-local 0600 files wiped in an EXIT trap, and the reviewer runs no `go build` of its own. The shared setup-jfrog exported JFROG_ACCESS_TOKEN + GOPROXY to $GITHUB_ENV and wrote a tokened ~/.netrc, all persisting into the model-driven Run reviewer step. - Engineer author + followup: keep setup-jfrog (the build needs the Go module proxy) but expand the build step to warm ALL caches while creds are live — `go build ./...` + `make tools` (bin/gotestsum + bin/golangci-lint, which the agent's make test/make lint build) + `go test -run '^$' ./...` (compiles every test binary, pulling test-only deps). Then a new "Scrub JFrog credentials" step removes ~/.netrc, blanks JFROG_ACCESS_TOKEN, and pins GOPROXY=off + GOFLAGS=-mod=readonly, so the agent runs entirely from the warm module cache and a cache miss fails loudly instead of reaching the network or editing go.mod/go.sum. Verified locally: after the warm step, an offline (GOPROXY=off) go test compiles and runs green. The shared .github/actions/setup-jfrog composite is untouched (real CI depends on its $GITHUB_ENV exports). Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
Addresses: - #3655298792 at pecotesting_creds_test.go:31 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Addresses review nit: the changelog line enumerated internal driver category names, which is too low-level for an external changelog. Co-authored-by: Isaac Signed-off-by: Prathamesh Baviskar <prathamesh.baviskar@databricks.com>
Addresses review nit: the changelog line enumerated internal driver category names, which is too low-level for an external changelog. Co-authored-by: Isaac Signed-off-by: Prathamesh Baviskar <prathamesh.baviskar@databricks.com>
Addresses review nit: the changelog line enumerated internal driver category names, which is too low-level for an external changelog. Co-authored-by: Isaac Signed-off-by: Prathamesh Baviskar <prathamesh.baviskar@databricks.com>
Adds a kernel-only connection option that scans top-level DECIMAL columns to a lossy float64 instead of the exact fixed-point string. The kernel still receives native Arrow Decimal128; only the Go scanner changes, skipping the per-cell string materialization for a cheap scalar. Off by default; mirrors the Thrift driver's pre-UseArrowNativeDecimal behavior. Nested decimals still render exactly. On decimal-heavy large results this is ~25% faster than the Thrift path and up to ~37% faster than the exact-string kernel path. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
TestKernelExperimentalFieldsClassified reflects over every KernelExperimentalConfig field and requires an explicit disposition entry so no experimental knob is silently dropped. Register DecimalAsFloat (wired Go-side via kernel.Config -> kernelOp -> arrowscan). Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
…mod=readonly) Addresses the reviewer-bot finding (PR #427, engineer-bot.yml:107): the comment said the step pins `GOFLAGS=-mod=mod`, but the code (line 116) correctly sets `-mod=readonly`. The code is right — readonly makes a cache miss fail loudly without editing go.mod/go.sum, which is the comment's own stated intent; `-mod=mod` would permit those edits, contradicting it. Comment-only fix to match the code (and the followup workflow, which already reads -mod=readonly). Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
…#427) ## Summary Onboards **both** [databricks-bot-engine](https://github.com/databricks/databricks-bot-engine) bots to `databricks-sql-go`, PAT-free (App-auth), pinned to engine SHA `d05dcb11`. Adapted for this repo's **Go / Makefile** toolchain. Structured to land in **one PR** (per the engine's one-PR onboarding checklist). - **reviewer-bot** — reviews every non-fork PR and posts inline findings. This repo has **no existing Claude automation**, so it's the sole autonomous reviewer — no trigger clash or duplicate coverage. - **engineer-bot** — given a maintainer-labelled **issue**, reproduces the bug with a failing **live `driver_e2e_test.go` test** against a warehouse (pure-Go Thrift backend), fixes the Go, and opens a PR; follows up on review comments. The engine is `pip`-installed in REF mode (it can't be `uses:`-d cross-repo) via two local composites that mirror the `databricks-sql-python` / `databricks-odbc` / `databricks-sql-nodejs` onboardings. ## What's in this PR | Path | Purpose | | --- | --- | | `.github/actions/install-bot-engine/` | REF-mode engine + Claude SDK/CLI install, PAT-free, JFrog-routed | | `.github/actions/bot-prelude/` | mint bot App token + engine-scoped token, Node, engine install. **Engine pin lives here** (single source of truth) | | `.github/workflows/reviewer-bot.yml` + `-followup.yml` | read-only; Python interpreter only | | `.github/workflows/engineer-bot.yml` + `-followup.yml` | Go 1.25 + JFrog GOPROXY + pure-Go build so the agent can build & test; author requires live e2e, followup runs `make test` only | | `.bot/config.yaml` + `.bot/prompts/**` | engineer config + prompts (Go); reviewer additive prompt | | `pecotesting_creds_test.go` | +token fallback so the e2e token survives the engine's env scrub | ## Go-specific adaptations - **Protected runner + JFrog mirror**, same model as the other internal drivers. `setup-jfrog` (Go module `GOPROXY`) runs before build/test; `id-token: write` on all jobs. - **Pure-Go path only.** The engineer builds `CGO_ENABLED=0` (the default backend), **not** the opt-in SEA/kernel path (`databricks_kernel` build tag + Rust + a private `databricks-sql-kernel` clone). The bug-fix prompt targets the Thrift-backed `driver_e2e_test.go`; kernel-only bugs report `blocked`. - **E2E silently skips without creds → token via config file.** `pecoTestingCreds` reads `DATABRICKS_PECOTESTING_TOKEN` and `t.Skip`s if it's empty — and the engine scrubs `*_TOKEN`-shaped env vars from the agent subprocess, so without a fallback the repro would silently skip and look green. The author workflow writes the token to `$RUNNER_TEMP/e2e-connection.json` and passes the path in `DATABRICKS_TEST_CONFIG_FILE` (a name the scrub preserves); `pecotesting_creds_test.go` falls back to it **only when the env token is absent**. Env var still wins → **normal CI and local dev are byte-for-byte unchanged**. (Verified: the edited file `gofmt -s`-clean and `go vet .` passes against the real deps.) - **No `go.sum` churn.** The `bash_allowlist` is `go build` / `go test` / `make test` / `make lint` / `gofmt -l -s .` / `git status` — it deliberately **excludes** `go get`, `go mod tidy` (would dirty `go.sum`), and `make fmt` (rewrites files in place; the agent formats via edits + checks with `gofmt -l`). - **`denied_subpaths`** blocks `.github/`, `build/` (kernel-lib scripts + native artifacts), and `testdata/` (fixtures). The prompts also warn off `go.mod`/`go.sum`, the depguard import allowlist, and the `stringer`-generated block in `internal/client/client.go`. ##⚠️ Prerequisites — provisioning YOU must do (a PR can't) The workflows will fail at the first token-mint step until these are in place (this is exactly what happened on the odbc onboarding until the Apps were installed): 1. **Two GitHub Apps**, each with `contents`/`pull-requests`/`issues: write`: - `REVIEW_BOT_APP_ID` + `REVIEW_BOT_APP_PRIVATE_KEY` - `ENGINEER_BOT_APP_ID` + `ENGINEER_BOT_APP_PRIVATE_KEY` (None exist in this repo today — only `INTEGRATION_TEST_APP_*`.) 2. **Install BOTH Apps on `databricks-sql-go` AND on `databricks-bot-engine`** (the latter with `contents: read`) — the engine install mints an engine-scoped token, so the App must see the engine repo. (Missing the repo install → `create-github-app-token` 404 at the prelude.) 3. Confirm `DATABRICKS_HOST` / `DATABRICKS_TOKEN` (used to build `MODEL_ENDPOINT` + the e2e token) point at a workspace hosting the `databricks-claude-opus-4-8` serving endpoint; `TEST_PECO_WAREHOUSE_HTTP_PATH` present (used for `DATABRICKS_PECOTESTING_HTTP_PATH2`). These must be visible to the `azure-prod` environment the author job uses. 4. A maintainer-only **`engineer-bot` label** has been created on this repo as part of this change. The reviewer needs no label — it fires on PR open. 5. `issues` / `workflow_dispatch` triggers only register from the **default branch**, so the engineer workflows go live only **after this PR merges**. ## Verification - All 4 workflows + 2 composites + `.bot/config.yaml` parse; **actionlint clean** (only the known `linux-ubuntu-latest` self-hosted-label false positive, same label `go.yml` uses). - `pecotesting_creds_test.go` is **`gofmt -s`-clean** and **`go vet .` passes** against the real module deps (fetched via the Go proxy). - Config has required `name`/`marker_namespace`/`pr_body_template`; fork gates + `id-token: write` present on all applicable jobs; no reuse of the required check names `Lint` / `Test and Build` / `Go Integration Tests`. - **First real end-to-end run happens post-merge** (protected runner + warehouse aren't reachable from a review env): after merging + provisioning, open a test PR (reviewer) and label a `Bug`-typed issue `engineer-bot` (author). This pull request and its description were written by Isaac. --- <!-- GITHUB_MCP_FOOTER: This attribution is automatically appended by GitHub MCP. --> _This PR was created with [GitHub MCP](http://go/mcps)._
- Carve out the decimal-as-float opt-in in the top-level-DECIMAL parity invariant comment so it no longer reads as violated. - Note in coltype.go that RawBytes scan type + float64 value under decimal-as-float is a deliberate Thrift-matching exception. - Add WithKernelDecimalAsFloat to the Thrift-reject and set-experimental option tables for per-option coverage. - Fix the stale CGO_ENABLED=0 build-tag comment in spog_routing_test. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Co-authored-by: Isaac
peco-review-bot F1 (Low): kernelBatchIterator.HasNext() returned true forever after a non-EOF fetch error — err was sticky and done was never set, so a caller looping on HasNext() that logs-and-continues (instead of breaking) on error would spin forever, with Next() re-returning the same error and a nil record. Surface the error exactly once from Next(), then mark done so HasNext() goes false and a further Next() returns io.EOF. The failed stream is still recorded via r.iterationErr for OnClose telemetry. Updates the sibling test (which pinned the old sticky behavior) and adds a termination regression test. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
… opt-in + U2M scopes/telemetry + SPOG warehouse-id routing (#416) ## What SEA-via-kernel↔Thrift parity fixes surfaced by the Go comparator (two `database/sql` connections against the same warehouse, differing only in `useKernel`), plus the public Arrow batch API, U2M scope/browser fixes, an auth-logging cleanup, and SPOG routing. All kernel-path-only and entirely Go-side (no kernel/C-ABI change). **Parity fixes** - **TIMESTAMP out-of-nanosecond-range wrapping.** Arrow's `ToTime` forms an `int64`-ns intermediate that overflows for microsecond TIMESTAMPs outside ~1678–2262 (`'0001-01-01'`→`1754-08-30`). Now uses the unit-specific `time.Unix*` constructors, covering 0001–9999. Thrift unaffected (gets a preformatted string). - **DDL affected-rows `-1` vs `0`.** The C ABI returns `-1` for unknown/NA counts (DDL/SELECT); Thrift reports `0`. Fold the `-1` sentinel to `0`; real DML counts pass through. - **User-Agent not forwarded.** The kernel's built-in UA leaked into query history. Now forwards the driver UA via `set_custom_header`, matching Thrift; `WithUserAgentEntry` still customizes. - **VOID/NULL columns typed `NULL` vs `STRING`.** Server stringifies VOID over the wire (even `SELECT NULL` reports `STRING_TYPE`), so `arrow.NULL` now maps to `STRING`, matching Thrift. Corrects two tests that pinned `NULL_TYPE` from the enum name, not live. **Arrow batch API** - **`GetArrowBatches` not exposed on kernel.** `kernelRows` now implements it over the zero-copy `next_batch` pull (prefetch for exact `HasNext`; transfer record ownership to the caller). `GetArrowIPCStreams` returns `ErrNotSupportedByKernel` (kernel exports via C Data Interface, not IPC). Compile-time `rows.Rows` assertion guards the gap. **Decimal-as-float opt-in (`WithKernelDecimalAsFloat`)** - New kernel-only connection option that scans **top-level DECIMAL** columns to a lossy `float64` (arrow-go `Num.ToFloat64`) instead of the exact fixed-point string. The kernel still receives native Arrow `Decimal128` over the wire — only the Go scanner changes, skipping the per-cell string materialization for a cheap scalar. **Off by default** (behavior byte-identical to today when unset); mirrors the Thrift driver's pre-`UseArrowNativeDecimal` default, and is rejected on the Thrift path (use `WithArrowNativeDecimal` there). Nested decimals (inside list/struct/map) still render exactly. - **Why:** the kernel's fetch/decode transport is already the faster of the two backends; its only large-result deficit was the per-cell decimal string render. Skipping it surfaces the transport win — on decimal-heavy large results (`tpcds_sf100.catalog_sales`, 15×`DECIMAL(7,2)`, 1.3M–31M rows) the option is **~25% faster than Thrift** (0.75–0.76×) and **up to ~37% faster than the exact-string kernel path** (0.63×), consistently across sizes and reps. Lossy beyond ~15–17 digits, so it's an opt-in speed knob, not a default. **U2M** - **Scope parity.** Kernel U2M forwarded no scopes → kernel applied `all-apis`, rejected where the built-in client lacks it. `resolveKernelAuth` now populates `Auth.Scopes` from the same `oauth.GetScopes` Thrift uses. - **Double-browser fix.** The telemetry/feature-flag client wrapped the interactive authenticator, launching a second browser at connect (blocking up to ~120s). Telemetry init is now skipped on the kernel+U2M path (detected via the `U2MClientID` structural interface); best-effort telemetry is dropped rather than made to prompt, matching Python's default. Unauthenticated telemetry is the follow-up — **PECOBLR-3839**. **Auth logging** - M2M/U2M/OAuth-config/token-provider paths logged through global `zerolog/log` (default Debug), ignoring `WithLogLevel` on both backends. Routed through the driver `logger` so they honor the configured level; auth logic untouched. **SPOG routing (`WithWarehouseID` path)** - On a unified/SPOG host the kernel needs `x-databricks-org-id`, which it injects **only** via `set_http_path` → `from_http_path` (parses the warehouse id + `?o=`). But Go calls `set_warehouse` (→ `ConnectionConfig::new`, no org id) whenever a warehouse id is set, and the kernel refuses a caller-supplied org-id header — so a `WithWarehouseID` SPOG session 303'd to `/login`. Now, when a warehouse id is set **and** the HTTP path is a canonical `warehouses`/`endpoints` path with a non-empty `?o=`, route via `set_http_path` (kernel parses the same id plus the org id). Guarded by `httpPathCarriesOrgRouting`, so non-SPOG connections keep the existing `set_warehouse` routing untouched. **Go-only** — Python/Node/ODBC always use the `http_path` route. ## Testing - Unit: `TestScanCellTimestampOutOfNanoRange`, `TestNormalizeAffectedRows`, `TestColumnTypeInfoFor`/`…MatchesThriftMapping`/`…ScanTypeCoversScanner` (VOID→STRING), `TestScanCellDecimalAsFloat` (float64 arm vs unchanged exact-string default), U2M scope assertions in `TestValidateKernelConfig`/`TestResolveKernelAuthRealAuthenticators`, `TestInteractiveU2MAuthenticatorDetection`, `TestHTTPPathCarriesOrgRouting`. `./auth/...` green after the logger swap (no import cycle). - Live (staging kernel backend): `GetArrowBatches` — 100,000 rows / 32 zero-copy batches, schema + count pass; `GetArrowIPCStreams` returns the sentinel. U2M double-browser prompt confirmed gone. **SPOG live on `peco.azuredatabricks.net`** — the `WithWarehouseID` routing that previously 303'd now returns `SELECT 1 == 1` / `current_user()` with the org-id header injected; non-SPOG queries unchanged. `WithKernelDecimalAsFloat` verified live: default returns `DECIMAL`/`string`, option-on returns `STRING`/`float64` for the same value, both `99.98`. - Both build paths green: pure-Go (`CGO_ENABLED=0`) and tagged cgo (`-tags databricks_kernel`). - **Comparator** (Thrift vs SEA): **9 diffs** (from 13 pre-fix, vs `main`'s 41). The 5 `dt_interval_column` metadata diffs are fixed here (INTERVAL_DAY_TIME→STRING). Of the 9 remaining, 8 are not driver bugs (NaN==NaN artifact ×3, non-deterministic EXPLAIN node ids, by-design volume-staging rejection, raw-`fetch_arrow` shape ×2, a permission-gated CREATE PROCEDURE `sqlState` detail); 1 is a deterministic `DESCRIBE TABLE` extra-trailing-rows diff that's server output (likely a comparator-side `skip_rows` filter, not this repo). Native decimal/interval on the raw-`fetch_arrow` path is a tradeoff, tracked as **PECOBLR-3838** (needs a kernel `KERNEL_REV` bump). Stacked on `mani/sea-kernel-new-items` (#412). Co-authored-by: Isaac
Reruns the existing TestKernelE2E* funcs against the Reyden SEA reference engine (TEST_WAREHOUSE=reyden + DATABRICKS_REYDEN_HTTP_PATH), routed via kernelTestDBWith. The step is continue-on-error since Reyden can be down or diverge from dbsql, and reyden-only divergences self-skip via skipOnReyden. The reyden path comes solely from the TEST_REYDEN_WAREHOUSE_HTTP_PATH secret; with it unset the leg is inert and re-runs on the normal warehouse. Adds TestIsReydenLeg + TestSkipOnReyden: pure-env unit tests (no warehouse) for the leg selector (case-insensitive, path-gated, fail-safe when unset) and the skip gate, so the routing logic runs in the kernel unit-test job. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
## What Add a **non-blocking Reyden E2E leg** to the nightly. Reyden is the SEA reference engine — the same SEA-via-kernel path as the existing kernel E2E suite, just pointed at a read-only reference warehouse over shared Unity Catalog. Rather than duplicate the 18 `TestKernelE2E*` funcs, this **reuses them verbatim** and routes the connection to Reyden when selected. - **`kernel_reyden_e2e_test.go`** (new): `isReydenLeg()` (`TEST_WAREHOUSE=reyden` + a reyden path in `DATABRICKS_REYDEN_HTTP_PATH`), `reydenHTTPPath()`, and `skipOnReyden()` — a gate for surfaces Reyden genuinely doesn't support. Inert unless the reyden env is set, so every existing run is unaffected. - **`kernel_e2e_test.go`**: `kernelTestDBWith` routes `WithHTTPPath` to the reyden warehouse on the reyden leg (same host + PAT). The `variant` data-type subcase skips on reyden — it cannot decode VARIANT over inline Arrow (a reyden-side gap; dbsql-sea and Thrift read it fine). No DDL/DML in this suite, so nothing else needs gating. - **`nightly-e2e.yml`**: a second, **non-blocking** (`continue-on-error`) run step in the existing `kernel-e2e` job re-runs the kernel E2E funcs with `TEST_WAREHOUSE=reyden` — the kernel lib is already built, so it's just another `go test`. The Thrift-vs-kernel parity funcs are excluded (they compare the kernel against Thrift on one warehouse; meaningless cross-warehouse). ## Why Give the SEA/kernel path continuous coverage against the Reyden reference engine, surfacing reyden-vs-dbsql divergences. `continue-on-error` because Reyden is a reference engine that can be down or diverge — a reyden-side failure must not fail the nightly (it surfaces in the step log). This mirrors the non-blocking Reyden integration leg in the shared driver-test suite. `DATABRICKS_REYDEN_HTTP_PATH` falls back to the well-known reyden warehouse id on the same host when the secret is unset; if it and `TEST_WAREHOUSE` resolve empty, the suite simply runs against the normal warehouse (`isReydenLeg()==false`) — a harmless no-op. ## Testing - `gofmt` clean; `go vet` clean on the `-tags databricks_kernel` build. - Leg-selection (`isReydenLeg`, fail-safe when path unset, case-insensitive) and the `skipOnReyden` gate unit-tested. - Nightly YAML parses. Live reyden read assertions pending a healthy reyden endpoint (the staging one is currently returning 500s); routing + skip behavior verified locally.
…417) ## What Attaches source-declared error categories at the error sites on the result-materialization paths, so telemetry reports a precise `error_name` instead of the generic `error` fallback: | Category | Site | |----------|------| | `chunk_download_error` | CloudFetch batch download failures (`internal/rows/arrowbased/batchloader.go`) | | `arrow_schema_parsing_error` | Arrow schema convert / serialize / read failures (`internal/rows/arrowbased/arrowRows.go`) | | `result_set_error` | result-page fetch (`internal/rows/rowscanner/resultPageIterator.go`) and result-set metadata fetch (`internal/rows/rows.go`) | | `statement_execution_timeout` | sentinel `WatchTimeout` in the poll loop (`internal/backend/thrift/backend.go`) | A fetch aborted via the results context (e.g. on Close) is left **untagged**, so it still classifies as `cancelled`/`timeout` rather than `result_set_error`, matching the CloudFetch path. `statement_execution_timeout` is unreachable in production today (the sole `Watch` call uses `timeout=0`, so the branch never fires and emits no telemetry); it is tagged and commented so it classifies correctly if a nonzero poll timeout is ever enabled. ## Why it's safe - All tags use the existing `WithCategory` chaining, which returns the **same concrete pointer**, so `errors.Is`/`errors.As`, the `dbsqlerr.DBError` assertion in the Arrow scan path, `Error()` strings, and sentinel identity are all unchanged. - Each live tag reaches `classifyError` via the row-iteration path (`Next` → `iterationErr` → `AfterExecute`), verified by the added tests. ## Also: a latent bug fix `rows.getResultSetSchema` wrapped the wrong (nil) variable on a `GetResultSetMetadata` failure, dropping the real cause from the error chain. Fixed to wrap the actual cause. This doesn't change the telemetry category (the tag wins regardless), but it restores `Cause()`/`Unwrap()`. --- Design doc: https://docs.google.com/document/d/12ufP1eZrgFxWt6xzINfkhfrE-NnhB29zCa5PKokVfp8/edit Jira: PECOBLR-3537
## What Tags the following error sources so telemetry reports a precise `error_name` instead of the generic `error` fallback: | Category | Site | |----------|------| | `unsupported_operation` | staging default case — server returns an op other than PUT/GET/REMOVE (`connection.go`) | | `decompression_error` | CloudFetch LZ4 decompression failure (`internal/rows/arrowbased/batchloader.go`) | | `execute_statement_cancelled` | query cancelled during status polling, `context.Canceled` only (`internal/backend/thrift/backend.go`) | `decompression_error` wraps the raw lz4 error through a driver constructor (Group B pattern) — previously it was re-wrapped downstream into a misleading "row number not contained" message and misclassified. `execute_statement_cancelled` tags **only** `context.Canceled`; `context.DeadlineExceeded` is left untagged so it keeps its existing classification. ## Why it's safe - All tags use the existing `WithCategory` chaining (same concrete pointer), so `errors.Is`/`errors.As`, the `dbsqlerr.DBError` assertion in the Arrow scan path, `Error()` strings, and sentinel identity are unchanged. The cancellation wrap preserves `errors.Is(err, context.Canceled)` for the thrift layer. - Each tag reaches `classifyError` on a verified live path; tests cover the decompression tag end-to-end through the batch iterator and the cancel-vs-deadline branch in `pollOperation`. ## Out of scope The arrow-IPC schema-parse errors in `arrowRecordIterator` are deliberately left untagged, that's the public `GetArrowBatches` path, whose errors go straight to the application and never reach `classifyError`. --- Design doc: https://docs.google.com/document/d/12ufP1eZrgFxWt6xzINfkhfrE-NnhB29zCa5PKokVfp8/edit Jira: PECOBLR-3537
…ry (#424) ## What Tags the remaining live error sources so telemetry reports a specific `error_name` instead of the generic `error`: | Category | Site | |----------|------| | `session_closed` | failed `CloseSession` (`connection.go`, DELETE_SESSION telemetry) | | `statement_closed` | failed `CloseOperation` RPC (`connection.go`, the 3 CLOSE_STATEMENT sites) | | `rate_limit_exceeded` | HTTP 429 that survives all retries (`internal/client/client.go` retry `errorHandler`) | All tags are **telemetry-only** and use the existing `WithCategory` chaining (same concrete pointer), so `errors.Is`/`errors.As`, `Error()` strings, and the errors returned to callers are unchanged: - `session_closed` tags a separate telemetry copy; the returned error stays a `driver.ErrBadConn` (`database/sql` relies on that for connection-pool eviction). - `statement_closed` tags only the telemetry argument; the raw close error still flows to the caller/rows. - `rate_limit_exceeded` is tagged in the retry `errorHandler` — the only place the 429 status is visible (net/http discards the response on a transport error). Scoped to 429 exactly, so 503 keeps its existing classification. This covers the retry-exhausted failure only; a per-attempt 429 counter metric is a TODO follow-up. ## Notes for reviewers (by design) - `statement_closed` is only live on the Thrift backend; the kernel backend's close never returns an error, so it can't be tagged there. - Because the innermost source category wins, a close that fails specifically due to a 429 reports `rate_limit_exceeded` rather than `session_closed`/`statement_closed` — the more specific category. ## Naming note `session_closed` has no exact JDBC equivalent (JDBC's nearest is `CONNECTION_CLOSED`); kept Go-specific here. --- Design doc: https://docs.google.com/document/d/12ufP1eZrgFxWt6xzINfkhfrE-NnhB29zCa5PKokVfp8/edit Jira: PECOBLR-3537
Restructure the README around the two execution backends (Thrift default, SEA/kernel opt-in), following the adbc-drivers/databricks C# README model: a protocol-selection section, a Building section covering the cgo + Rust static-lib build, and connection-property tables whose Protocol column marks each parameter Both / Thrift-only / SEA-only. Add an examples/kernel example for the SEA/kernel backend. Also fix two stale/inaccurate comments found while writing the docs: - doc.go: the U2M default scopes are now at parity with Thrift (the kernel path forwards oauth.GetScopes), not the older all-apis + offline_access set. - internal/config/config.go: the telemetry batch-size / flush-interval default comments said 100 / 5s, but DefaultConfig() resolves them to 200 / 30s. Addresses PECOBLR-3737. Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
… rejected Address peco-review-bot: the SEA-only legend bullet said such params are rejected on Thrift with ErrRequiresKernelBackend, but that only holds for the WithKernel* options (which allocate KernelExperimental and hit the connector reject gate). warehouseId sets a plain UserConfig field, so Thrift silently ignores it. Carve out that exception in the legend. Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
The Telemetry prose claimed telemetry is 'disabled by default and requires explicit opt-in', contradicting the table row (and source). isTelemetryEnabled returns the server feature-flag decision when enableTelemetry is unset (the default), so telemetry can be active without an explicit opt-in; an explicit enableTelemetry=true/false overrides the flag rather than respecting it. Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
…telemetry skip Address code-review-squad findings on #428: - WithMaxRows godoc said 'Default is 10000' but defaultMaxRows is 100000; WithCloudFetch said 'Default is false' but WithDefaults sets it true. Both now contradicted the new README default tables (which route readers to connector.go). Fix the two godocs to match source. - Telemetry section claimed it 'applies to both backends'; note the one exception — kernel backend + OAuth U2M skips telemetry (connector.go:101-107) to avoid a second browser flow, regardless of enableTelemetry. Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Close AC #2 completeness gaps: - WithTransport (exported ConnOption) had no property-table row. It is Thrift-only — rejected on the kernel path (kernel_config.go:67, wraps ErrNotSupportedByKernel) since the kernel uses its own HTTP stack. Add a row to the TLS table pointing kernel users at WithKernelTrustedCerts/WithKernelProxy. - Document telemetry_retry_count / telemetry_retry_delay as deprecated-and-ignored DSN params (config.go:489-498), matching the ADBC reference's treatment of deprecated params. Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
## What Documents how every connection/config parameter behaves across the driver's two execution backends — Thrift (default) and the opt-in SEA/kernel backend — so parameter drift between protocols is visible. Follows the [`adbc-drivers/databricks` C# README](https://github.com/adbc-drivers/databricks/blob/main/csharp/README.md#databricks-specific-properties) model referenced in PECOBLR-3737. - **README.md** — restructured around the two backends: - a **protocol-selection** section (`WithUseKernel` / `useKernel=true`, and the fail-loud `ErrKernelNotCompiled` behavior), - a **Building** section covering the difference between the pure-Go Thrift build and the cgo + Rust static-lib kernel build (`make kernel-lib` / `build-kernel` / `test-kernel`, cross-compile caveats), - **connection-property tables** whose **Protocol** column marks each parameter `Both` / `Thrift-only` / `SEA-only`, and a call-out of every protocol-specific parameter and behavior/default difference. - **examples/kernel/main.go** — a runnable example of the SEA/kernel backend (`WithUseKernel`, `WithWarehouseID`, and `errors.Is` detection of `ErrKernelNotCompiled` / `ErrNotSupportedByKernel`). Compiles under the default pure-Go build; requires a `-tags databricks_kernel`, `CGO_ENABLED=1` build to actually select the kernel. ## Incidental fixes (found while writing the docs) - **doc.go** — the U2M default scopes are now at parity with Thrift (the kernel path forwards `oauth.GetScopes`); the prose still described the older `all-apis + offline_access` set. - **internal/config/config.go** — the `TelemetryBatchSize` / `TelemetryFlushInterval` default comments said `100` / `5s`, but `DefaultConfig()` resolves an unset value to `200` / `30s`. Comment-only correction. ## Notes for reviewers - Docs + one example + two comment corrections. No behavior change. - Every documented default was verified against source (`config.go`, `connector.go`, `kernel_config.go`, `Makefile`), not carried over from the old README. Addresses PECOBLR-3737. This pull request and its description were written by Isaac.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sync fork.