Conversation
- Add dbt-vertica as optional dependency in pyproject.toml - Add Vertica transient error patterns for automatic retry - Add Vertica Docker service to e2e test docker-compose - Add Vertica profile to test profiles template - Add Vertica to CI workflow matrix (test-warehouse + test-all-warehouses) - Handle dbt-vertica install with --no-deps (pins dbt-core~=1.8) Co-Authored-By: Itamar Hartstein <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
👋 @devin-ai-integration[bot] |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds Vertica support across CI/workflows, dependency extras, test Docker infrastructure and profiles, transient-error detection, and a Vertica-specific dbt seed macro and install/workflow adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Runner as GitHub Runner
participant Docker as Docker Compose
participant Installer as dbt Installer
participant Vertica as Vertica DB
CI->>Runner: trigger workflow (matrix includes vertica)
Runner->>Docker: start containers (incl. vertica service)
Docker->>Vertica: start & run healthcheck
Runner->>Installer: run install steps (conditional vertica flow)
Installer->>Vertica: install/connect (dbt-vertica path)
Runner->>Installer: run dbt commands/tests
Vertica-->>Installer: responses (or transient errors matched)
Runner->>CI: collect and report results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The dbt-vertica package pins dbt-core~=1.8. Installing elementary with the [vertica] extra would re-resolve dbt-vertica's deps and downgrade dbt-core, breaking package-lock.yml parsing and protobuf compat. Fix: install elementary without the adapter extra for Vertica (dbt-vertica is already present via --no-deps), and explicitly install vertica-python. Co-Authored-By: Itamar Hartstein <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/test-all-warehouses.yml (1)
89-111:⚠️ Potential issue | 🟠 MajorGate the Vertica matrix leg until the companion dbt package ref is available.
This now runs
verticaon every PR, butdbt-data-reliability-refis still empty for the automatic triggers. Per the PR notes, that means this leg will keep checking out a non-Vertica-compatible branch ofelementary-data/dbt-data-reliabilityand fail deterministically until the companion PR merges. Either excludeverticafrom automatic PR runs for now, or inject the companion ref by default for this matrix entry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test-all-warehouses.yml around lines 89 - 111, The vertica matrix entry is being run on automatic PRs without a companion dbt-data-reliability-ref; update the warehouse-type matrix to either (A) exclude 'vertica' when github.event_name is 'pull_request' or 'pull_request_target' by filtering the matrix.list (i.e., conditionally build the warehouse-type array to omit 'vertica' for automatic PRs), or (B) inject a default companion ref for the vertica entry by setting dbt-data-reliability-ref to a non-empty default when matrix.warehouse-type == 'vertica' (use the existing inputs.dbt-data-reliability-ref or a lookup for vertica-specific default); adjust the workflow expression that defines warehouse-type and/or the uses: ./.github/workflows/test-warehouse.yml with the dbt-data-reliability-ref input so the vertica leg is skipped or given a valid ref for automatic PR triggers..github/workflows/test-warehouse.yml (1)
201-214:⚠️ Potential issue | 🔴 CriticalPin a default dbt-core version for the Vertica path.
When
inputs.dbt-versionis empty—as it is in the default matrix—Line 208 becomespip install dbt-core, which pulls version 1.11.5 (latest), butdbt-verticarequiresdbt-core==1.8.5exactly. The--no-depsworkaround on Line 207 becomes ineffective, leaving the Vertica test leg with incompatible dependencies. Pin a default version like1.8.5on Line 208 to match dbt-vertica's requirement:Suggested fix
- name: Install dbt (Vertica) if: inputs.warehouse-type == 'vertica' run: | pip install --no-deps dbt-vertica - pip install "dbt-core${{ inputs.dbt-version && format('=={0}', inputs.dbt-version) }}" + pip install "dbt-core${{ inputs.dbt-version && format('=={0}', inputs.dbt-version) || '==1.8.5' }}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test-warehouse.yml around lines 201 - 214, The Vertica job installs dbt-core without a fallback so when inputs.dbt-version is empty it pulls a too-new version; update the "Install dbt (Vertica)" step to pin a default dbt-core version (e.g. 1.8.5) when inputs.dbt-version is not provided by changing the pip install invocation that currently uses inputs.dbt-version interpolation to use a fallback value (inputs.dbt-version || '1.8.5'); ensure this change affects the pip install "dbt-core..." call referenced in the "Install dbt (Vertica)" step so dbt-vertica remains compatible.
🧹 Nitpick comments (1)
tests/e2e_dbt_project/docker-compose.yml (1)
293-321: Pin the Vertica image to a tested tag or digest.
ghcr.io/ratiopbc/vertica-ceis currently floating on the registry default tag, so CI behavior can change underneath this workflow and the third-party dependency review is hard to reproduce. Please lock this to the exact image version/digest you validated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e_dbt_project/docker-compose.yml` around lines 293 - 321, The Vertica service is using an unpinned image reference causing non-reproducible CI runs; update the vertica service's image field (the "image" property under the vertica service) to a specific, tested tag or immutable digest (e.g., ghcr.io/ratiopbc/vertica-ce:<tested-tag> or ghcr.io/ratiopbc/vertica-ce@sha256:<digest>) and commit that change in docker-compose.yml so CI pulls the exact validated image; optionally add a short comment noting the tested tag/digest and verification date.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@elementary/clients/dbt/transient_errors.py`:
- Around line 123-128: The Vertica transient-patterns tuple in the transient
error mapping contains an incorrect entry "no transaction in progress" that
should be removed; edit the mapping keyed by "vertica" in transient_errors.py
(the tuple currently with "connection timed out", "could not connect to the
server", "ssl syscall error", "no transaction in progress") and delete the
fourth element so the tuple only includes the three real transient patterns,
ensuring any substring-matching logic that uses this mapping (e.g., the
transient error checker that references the "vertica" key) will no longer treat
transaction-state notices as retryable.
---
Outside diff comments:
In @.github/workflows/test-all-warehouses.yml:
- Around line 89-111: The vertica matrix entry is being run on automatic PRs
without a companion dbt-data-reliability-ref; update the warehouse-type matrix
to either (A) exclude 'vertica' when github.event_name is 'pull_request' or
'pull_request_target' by filtering the matrix.list (i.e., conditionally build
the warehouse-type array to omit 'vertica' for automatic PRs), or (B) inject a
default companion ref for the vertica entry by setting dbt-data-reliability-ref
to a non-empty default when matrix.warehouse-type == 'vertica' (use the existing
inputs.dbt-data-reliability-ref or a lookup for vertica-specific default);
adjust the workflow expression that defines warehouse-type and/or the uses:
./.github/workflows/test-warehouse.yml with the dbt-data-reliability-ref input
so the vertica leg is skipped or given a valid ref for automatic PR triggers.
In @.github/workflows/test-warehouse.yml:
- Around line 201-214: The Vertica job installs dbt-core without a fallback so
when inputs.dbt-version is empty it pulls a too-new version; update the "Install
dbt (Vertica)" step to pin a default dbt-core version (e.g. 1.8.5) when
inputs.dbt-version is not provided by changing the pip install invocation that
currently uses inputs.dbt-version interpolation to use a fallback value
(inputs.dbt-version || '1.8.5'); ensure this change affects the pip install
"dbt-core..." call referenced in the "Install dbt (Vertica)" step so dbt-vertica
remains compatible.
---
Nitpick comments:
In `@tests/e2e_dbt_project/docker-compose.yml`:
- Around line 293-321: The Vertica service is using an unpinned image reference
causing non-reproducible CI runs; update the vertica service's image field (the
"image" property under the vertica service) to a specific, tested tag or
immutable digest (e.g., ghcr.io/ratiopbc/vertica-ce:<tested-tag> or
ghcr.io/ratiopbc/vertica-ce@sha256:<digest>) and commit that change in
docker-compose.yml so CI pulls the exact validated image; optionally add a short
comment noting the tested tag/digest and verification date.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40998330-3203-4356-b4b4-4c5fd2f6b77d
📒 Files selected for processing (6)
.github/workflows/test-all-warehouses.yml.github/workflows/test-warehouse.ymlelementary/clients/dbt/transient_errors.pypyproject.tomltests/e2e_dbt_project/docker-compose.ymltests/profiles/profiles.yml.j2
- Remove 'no transaction in progress' from Vertica transient patterns (it's a transaction-state notice, not a transient connection error) - Hardcode dbt-data-reliability-ref to 'vertica-compat' for the Vertica matrix entry until PR #963 merges into master Co-Authored-By: Itamar Hartstein <[email protected]>
PR #963 has been merged to master (2ab66fbe). Update: - packages.yml/package-lock.yml: point to merge commit on master - test-all-warehouses.yml: remove vertica-compat branch override Co-Authored-By: Itamar Hartstein <[email protected]>
…lision) dbt-vertica's COPY command hardcodes 'seed_rejects' as the reject table name for every seed, causing 'Object already exists' errors when dbt seed processes more than one file. This override (same as in dbt-data-reliability integration tests) uses per-seed reject table names. Co-Authored-By: Itamar Hartstein <[email protected]>
feat: add Vertica adapter support
Summary
Adds Vertica as a supported adapter in the elementary CLI, following the same patterns as existing adapters (Snowflake, BigQuery, Postgres, etc.). This is the companion PR to dbt-data-reliability#963 which adds Vertica support to the dbt package (now merged).
Changes:
pyproject.toml—dbt-vertica >=1.7,<2.0.0as optional dependency +verticaextratransient_errors.py— Vertica-specific retry patterns (connection timeout, SSL, etc.)docker-compose.yml— Vertica CE container (ghcr.io/ratiopbc/vertica-ce) for local/CI testingprofiles.yml.j2— Vertica dbt profile target + added toelementaryprofile targets listtest-warehouse.yml— Vertica start step, special--no-depsdbt-vertica install (workaround for itsdbt-core~=1.8pin), added to Docker adapter listtest-all-warehouses.yml— Addedverticato CI matrixpackages.yml/package-lock.yml— Updateddbt-data-reliabilityrevision to the merge commit of [ELE-1257] Support Dremio #963 on masterNo SQL macro changes were needed — Vertica uses standard SQL so the existing
default__dispatches should work.Updates since last revision:
vertica-compatbranch override intest-all-warehouses.ymland updatedpackages.yml/package-lock.ymlto reference the merge commit (2ab66fbe).Review & Testing Checklist for Human
--no-depsdbt-vertica install doesn't conflict withpip install ".[vertica]"— The workflow doespip install --no-deps dbt-verticathen laterpip install "."(without the vertica extra). Confirm the final environment has a working dbt-vertica + latest dbt-core.ghcr.io/ratiopbc/vertica-ceis maintained by Ratio PBC, not Vertica/OpenText. This is the same image used in dbt-data-reliability#963. Confirm this is acceptable or switch to an official image (requires Docker Hub auth).transient_errors.pywere inferred, not tested against actual Vertica error output. Verify these match real Vertica error messages.warehouse-type: verticato ensure everything works (seed → run → test → monitor → report).Notes
Summary by CodeRabbit