Skip to content

fix(ingestion): heal Decimal jira id columns on warm clusters (#1743) #1940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mitasovr merged 7 commits into from
Jul 27, 2026
Merged

fix(ingestion): heal Decimal jira id columns on warm clusters (#1743) #1940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
75dda6c
fix(ingestion): heal Decimal jira id columns on warm clusters (#1743)
cyberantonz Jul 27, 2026
6279b88
chore(bootstrap-db): document regen prerequisites; fix racy venv check
cyberantonz Jul 27, 2026
e21b15f
Merge branch 'main' into fix/1743-jira-id-decimal-heal
cyberantonz Jul 27, 2026
91d984e
Merge branch 'main' into fix/1743-jira-id-decimal-heal
cyberantonz Jul 27, 2026
a45ab84
Merge branch 'main' into fix/1743-jira-id-decimal-heal
cyberantonz Jul 27, 2026
f8d0d2a
Merge branch 'main' into fix/1743-jira-id-decimal-heal
cyberantonz Jul 27, 2026
b89284c
Merge branch 'main' into fix/1743-jira-id-decimal-heal
cyberantonz Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion .github/workflows/connectors-ddl-reminder.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,17 @@ jobs:
'',
'This PR changes `src/ingestion/**`. If your change affects any',
'bronze / silver / gold schema, regenerate the committed DDL snapshot',
'and include it in this PR:',
'and include it in this PR.',
'',
'Prerequisites (details: `src/ingestion/scripts/bootstrap-db/README.md`):',
'- docker + a fresh throwaway ClickHouse 25.7.5 (README "Local ClickHouse for testing")',
'- `.env` from `.env.bootstrap.example` pointing at it; use the host LAN IP,',
' reachable from both the host and connector containers',
' (`host.docker.internal` does not resolve on the macOS host itself)',
'- python3.12 or python3.11 on PATH (pinned dbt venv)',
'- HubSpot + Salesforce credentials in `.env` — their `discover` calls the',
' live APIs; without them, apply `scripts/connectors-ddl/{hubspot,salesforce}.sql`',
' to seed their bronze, then run the dbt step',
'',
'```bash',
'cd src/ingestion/scripts/bootstrap-db',
Expand Down
22 changes: 22 additions & 0 deletions src/ingestion/scripts/apply-ch-migrations.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,28 @@ SQL

heal_task_users_table silver class_task_users

echo "=== Healing jira task id column types (#1743) ==="
# #1892 retyped the jira staging id projections (worklog_id, comment_id)
# from raw bronze Decimal(38,9) to toString(...), but pre-existing
# incremental-append staging tables keep the Decimal column, and the
# positional union with the youtrack String twins fails with NO_COMMON_TYPE,
# blanking Task Delivery / Code Quality. MODIFY converges warm tables (and
# silver targets built from them) to the snapshot's String; Decimal->String
# is lossless. Guarded: staging tables exist only after the first jira sync.
heal_task_id_column() {
local db="$1" table="$2" column="$3"
ch_table_exists "$db" "$table" || return 0
echo " ${db}.${table}.${column}"
run_ch <<SQL
ALTER TABLE ${db}.${table} MODIFY COLUMN IF EXISTS ${column} Nullable(String);
SQL
}

heal_task_id_column staging jira__task_worklogs worklog_id
heal_task_id_column staging jira__task_comments comment_id
heal_task_id_column silver class_task_worklogs worklog_id
heal_task_id_column silver class_task_comments comment_id

echo "=== Reconciling legacy Bitbucket bronze placeholders (warm clusters) ==="
# Warm clusters still hold the pre-rewrite FLAT bronze_bitbucket_cloud.{commits,
# pull_requests}; the snapshot applicator's CREATE TABLE IF NOT EXISTS never
Expand Down
6 changes: 4 additions & 2 deletions src/ingestion/scripts/bootstrap-db/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ How it works: for every connector the source image runs `discover` (schemas are

## Prerequisites

- `docker`, `jq`, `yq` (mikefarah v4), `dbt` with `dbt-clickhouse`
- ClickHouse reachable under `CLICKHOUSE_HOST` both from this machine (dbt) and from inside docker containers (destination connector). For a ClickHouse running on this machine use `host.docker.internal`.
- `docker`, `jq`, `yq` (mikefarah v4)
- `python3.12` or `python3.11` on `PATH` — `run-dbt.sh` builds a local `.venv` with the pinned dbt from it (parity with the toolbox image; dbt-core 1.10 does not run on newer pythons). A `python -m venv`-capable interpreter is required (uv-managed pythons lack `ensurepip`; with those, pre-build the venv via `uv venv --seed .venv && .venv/bin/pip install dbt-core==<pin> dbt-clickhouse==<pin>`).
- ClickHouse reachable under `CLICKHOUSE_HOST` both from this machine (dbt) and from inside docker containers (destination connector). For a ClickHouse running on this machine use the machine's LAN IP (`ipconfig getifaddr en0`) — `host.docker.internal` resolves inside containers but not on the macOS host itself.

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make the ClickHouse host instructions consistent.

Line 11 now correctly says host-based dbt should use the machine LAN IP, but the local example at Line 25 still sets CLICKHOUSE_HOST=host.docker.internal and claims it works for dbt. On macOS, following that example makes host-side dbt unable to resolve ClickHouse; update the example to match this prerequisite or explicitly scope host.docker.internal to containers only.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ingestion/scripts/bootstrap-db/README.md` at line 11, Update the local
ClickHouse example in the README to use the machine’s LAN IP for host-based dbt,
matching the prerequisite guidance. If retaining host.docker.internal,
explicitly scope it to container-only usage and ensure the example no longer
claims it works for dbt.

- Real HubSpot and Salesforce credentials in `.env` — their CDK `discover` calls the live APIs, so fake values fail. Without credentials, seed their bronze from the committed snapshot instead: apply `scripts/connectors-ddl/{hubspot,salesforce}.sql`, run `./run-dbt.sh --select hubspot__bronze_promoted salesforce__bronze_promoted`, and continue from the dbt step.

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fix the fallback snapshot path consistently.

Both instructions use a repository-root-relative path while the documented bootstrap flow runs from src/ingestion/scripts/bootstrap-db; use ../connectors-ddl/{hubspot,salesforce}.sql from that directory, or make the working directory explicit.

  • src/ingestion/scripts/bootstrap-db/README.md#L12-L12: correct the fallback path in the prerequisites.
  • .github/workflows/connectors-ddl-reminder.yml#L58-L60: apply the same corrected path in the generated reminder.
📍 Affects 2 files
  • src/ingestion/scripts/bootstrap-db/README.md#L12-L12 (this comment)
  • .github/workflows/connectors-ddl-reminder.yml#L58-L60
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ingestion/scripts/bootstrap-db/README.md` at line 12, Correct the
fallback snapshot path in src/ingestion/scripts/bootstrap-db/README.md at lines
12-12 and the generated reminder in
.github/workflows/connectors-ddl-reminder.yml at lines 58-60 to use
../connectors-ddl/{hubspot,salesforce}.sql from the bootstrap-db working
directory, or explicitly set the working directory before using the
repository-root-relative path.


## Local ClickHouse for testing

Expand Down
5 changes: 4 additions & 1 deletion src/ingestion/scripts/bootstrap-db/run-dbt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ set +a

VENV_DIR="${SCRIPT_DIR}/.venv"
DBT_BIN="${VENV_DIR}/bin/dbt"
if [[ ! -x "${DBT_BIN}" ]] || ! "${VENV_DIR}/bin/pip" show dbt-clickhouse 2>/dev/null | grep -q "Version: ${DBT_CLICKHOUSE_VERSION}"; then
# No `pip show | grep -q`: grep -q exits at first match, pip dies on EPIPE and
# pipefail fails the check for a perfectly good venv (which then gets rm -rf'd).
INSTALLED="$("${VENV_DIR}/bin/pip" show dbt-clickhouse 2>/dev/null || true)"
if [[ ! -x "${DBT_BIN}" ]] || [[ "${INSTALLED}" != *"Version: ${DBT_CLICKHOUSE_VERSION}"* ]]; then
Comment on lines +19 to +22

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
fd -a 'run-dbt\.sh' . || true

echo "== file excerpt =="
if [ -f src/ingestion/scripts/bootstrap-db/run-dbt.sh ]; then
  nl -ba src/ingestion/scripts/bootstrap-db/run-dbt.sh | sed -n '1,80p'
fi

echo "== dbt version variables/usages =="
rg -n "DBT_(CLICKHOUSE|CORE)_VERSION|dbt-clickhouse|dbt-core|pip.*(show|install)" src/ingestion || true

echo "== deterministic behavior probe for bash substring matches =="
python3 - <<'PY'
import shlex, subprocess, sys

cases = [
    ("1.10.0", "1.10.0"),
    ("1.10.0rc1", "1.10.0"),
    ("1.10.0.post1", "1.10.0"),
    ("2.10.0", "1.10.0"),
    ("1.10.0", "1.10.0dev"),
]
for expected, actual in cases:
    installed = f'Name: dbt-clickhouse\nVersion: {actual}'
    bash = f'INSTALLED=$\'{installed.replace(chr(10), "\\n").replace("Name", "\\\\nName").replace("Version", "\\\\nVersion")}\'\n[[ $INSTALLED == *"Version: ${expected}"* ]]'
    res = subprocess.run(["bash","-c", bash], stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
    print(f"expected={expected!r} actual={actual!r} accepted={res.returncode==0}")
PY

Repository: constructorfabric/insight

Length of output: 305

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file excerpt =="
if [ -f src/ingestion/scripts/bootstrap-db/run-dbt.sh ]; then
  awk '{printf "%6d\t%s\n", NR, $0}' src/ingestion/scripts/bootstrap-db/run-dbt.sh | sed -n '1,90p'
fi

echo "== dbt version variables/usages =="
if command -v rg >/dev/null 2>&1; then
  rg -n "DBT_(CLICKHOUSE|CORE)_VERSION|dbt-clickhouse|dbt-core|pip.*(show|install)" src/ingestion || true
else
  grep -RInE "DBT_(CLICKHOUSE|CORE)_VERSION|dbt-clickhouse|dbt-core|pip.*(show|install)" src/ingestion || true
fi

echo "== deterministic behavior probe =="
python3 - <<'PY'
import subprocess

cases = [
    ("1.10.0", "1.10.0"),
    ("1.0.0", "1.10.0"),
    ("1.10.0rc1", "1.10.0"),
    ("1.10.0.post1", "1.10.0"),
    ("dbt-core1.10.0rc1", "1.10.0"),
]
for expected, actual in cases:
    installed = "Name: dbt-clickhouse\nVersion: " + actual
    bash = 'INSTALLED=$' + repr(installed) + '\n[[ "${INSTALLED}" != *"Version: ' + expected + '*" ]] && echo reject || echo accept'
    res = subprocess.run(["bash","-c", bash], stdin=subprocess.DEVNULL, text=True, stdout=subprocess.PIPE)
    print(f"expected={expected!r} actual={actual!r} returncode={res.returncode} output={res.stdout.strip()}")
PY

Repository: constructorfabric/insight

Length of output: 7176

Validate both pinned dbt versions with exact comparisons.

The current reuse guard only checks dbt-clickhouse, and Version: ${DBT_CLICKHOUSE_VERSION} is substring-matched, so incompatible values like 1.9.6.post1 could pass when the pin is 1.9.6. Also validate DBT_CORE_VERSION from pip show dbt-core with an exact version parse before reusing the venv.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ingestion/scripts/bootstrap-db/run-dbt.sh` around lines 19 - 22, Update
the virtualenv reuse guard around DBT_BIN and INSTALLED to query both
dbt-clickhouse and dbt-core, parse each package’s reported version exactly, and
require exact equality with DBT_CLICKHOUSE_VERSION and DBT_CORE_VERSION.
Recreate the environment when either package is missing, either version differs,
or DBT_BIN is unavailable; do not rely on substring matching.

PYTHON_BIN="$(command -v python3.12 || command -v python3.11)"
: "${PYTHON_BIN:?python3.12 or python3.11 is required to run dbt (same major as the toolbox image)}"
rm -rf "${VENV_DIR}"
Expand Down
Loading