feat(ingestion): bootstrap-db — generate connectors-ddl snapshot - #1892
Merged
mitasovr merged 14 commits intoJul 27, 2026
Conversation
Replace the hand-written create-bronze-placeholders.sh (ADR-0007) with a generated DDL snapshot in scripts/connectors-ddl/*.sql, dumped by bootstrap-db from real connectors + dbt + migrations; create-bronze-placeholders.sh becomes a thin applicator so a fresh cluster type-checks every gold view and builds end-to-end (constructorfabric#1831, constructorfabric#1763). All environments are on ClickHouse 25.7.5. Generation / prod: - Order: create person/identity (init-identity) BEFORE dbt so the identity seed models' ANTI JOIN person.persons resolves; gold-view migrations after dbt. - CH 25.7: allow_experimental_correlated_subqueries at PROFILE level in all four dbt profile generators; identity seed join key cast to toUUID (String vs UUID). - dump-ddl emits person/identity + only the gold-referenced staging tables (m365__collab_email_activity) — NOT all of staging: pre-creating an incremental staging model empty poisons its first is_incremental build (max(date) FROM this over an empty MergeTree reads the Date type-max on 25.7). - SKIP_DBT_GOLD skips apply-ch-migrations' own gold build during generation. - Warm-cluster Bitbucket bronze reconcile (constructorfabric#1880) relocated into apply-ch-migrations heals. - Contract-bug fixes surfaced by the real-schema snapshot (github-v2/gitlab/jira dbt models, crm/m365/ai casts, depends_on hints). e2e rig: _ensure_table allows the real table to be a superset of the fixture; seeder Decimal coercion + jira fixture alignment; declarative skip:; git_metrics skipped pending a github-v2 fixture refresh. Snapshot: 53 gold views (incl git_metric_observations), all class_* incl git, person.persons. e2e green: metrics 34 passed/1 skipped, api 64 passed/12 xfail. Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR replaces hand-written ClickHouse placeholders with committed DDL snapshots, updates bootstrap and e2e migration ordering, aligns ingestion model types and dependencies, enables ClickHouse 25.7 settings, and adds connector schemas, fixtures, documentation, and CI reminder support. ChangesSnapshot-driven ClickHouse bootstrap
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mitasovr
marked this pull request as ready for review
July 24, 2026 05:34
mitasovr
changed the title
mitasovr
force-pushed
the
claude/connectors-ddl-1831
branch
from
July 24, 2026 05:44
344df2f to
d92b3d5
Compare
The snapshot (scripts/connectors-ddl/*.sql) is regenerated MANUALLY for now (bootstrap-db.sh + dump-ddl.sh) — the auto-regenerate-and-commit workflow is dropped. Add a lightweight reminder that posts a single sticky comment when a PR touches src/ingestion, nudging the author to regenerate and commit the snapshot. Uses pull_request_target (writable token even on fork PRs) but never checks out or runs PR code and uses no secrets, so it is safe. No regeneration or push happens in CI. Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
mitasovr
force-pushed
the
claude/connectors-ddl-1831
branch
from
July 24, 2026 07:17
d92b3d5 to
96cc3aa
Compare
mitasovr
marked this pull request as draft
July 24, 2026 07:25
mitasovr
marked this pull request as ready for review
July 24, 2026 07:26
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
src/ingestion/tests/e2e/lib/dbt_runner.py (1)
298-307: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the obsolete refreshable MV profile flag and clean up stale SQL flags.
allow_experimental_refreshable_materialized_viewis obsolete in ClickHouse 25.7.5 and only produces an obsolete-setting warning/no-op; refreshable MVs are already GA. This e2e profile now only needsallow_experimental_correlated_subqueries, so remove the RMV flag fromsrc/ingestion/tests/e2e/lib/dbt_runner.pyand update remaining SQL settings that still enable the obsolete RMV flag.🤖 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/tests/e2e/lib/dbt_runner.py` around lines 298 - 307, Remove allow_experimental_refreshable_materialized_view from the settings mapping in dbt_runner.py, and remove any remaining SQL settings that enable the same obsolete flag. Keep allow_experimental_correlated_subqueries and all other required settings unchanged..github/workflows/connectors-ddl-reminder.yml (1)
6-9: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin
actions/github-scriptto an immutable commit SHA.This workflow runs on untrusted PR events with write permission. Although it does not execute PR code, the mutable
@v7reference could change underneath the workflow and gain repository write access. Pin the action to a full commit SHA and update it deliberately.Also applies to: 33-33
🤖 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 @.github/workflows/connectors-ddl-reminder.yml around lines 6 - 9, Pin the actions/github-script usage in the workflow to a full immutable commit SHA instead of the mutable `@v7` tag. Update only that action reference and retain its existing configuration and behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with 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.
Inline comments:
In @.claude/skills/metric-test/SKILL.md:
- Around line 305-306: Update the bootstrap guidance near the Bronze table
source description to remove instructions for adding DDL to bootstrap heredocs.
Replace them with the documented workflow for regenerating the CI-applied
connectors-ddl/*.sql snapshots, and instruct users to commit the regenerated
snapshot files so fixture tables are included.
In `@src/ingestion/scripts/bootstrap-db/bootstrap-db.sh`:
- Around line 62-73: Update the bootstrap flow around run-dbt.sh and
SKIP_DBT_GOLD so apply-ch-migrations.sh always rebuilds the gold models when the
dbt run fails. Track the run-dbt.sh exit status, set SKIP_DBT_GOLD=1 only after
a successful run, and preserve the existing continuation behavior while ensuring
a failed run cannot leave the gold rebuild skipped.
In `@src/ingestion/scripts/bootstrap-db/create-connector-tables.sh`:
- Line 90: Update the PROMOTED_FILE lookup in the connector table creation flow
to tolerate a missing "${CONNECTOR_DIR}/dbt" directory, preventing find’s
nonzero status from terminating the script under pipefail and set -e. Preserve
the existing behavior of selecting the first *__bronze_promoted.sql file and
allowing the subsequent promotion-skip logic to handle connectors without dbt
files.
In `@src/ingestion/scripts/bootstrap-db/dump-ddl.sh`:
- Around line 32-56: Replace each ClickHouse row-fetch process substitution in
dump_tables, dump_views, and the other per-connector,
person/identity/silver/insight, and staging loops with command substitution
captured in a variable before entering the loop, then feed that variable via a
here-string. Ensure ch() failures propagate to set -e while preserving the
existing row filtering and loop behavior.
In `@src/ingestion/scripts/bootstrap-db/README.md`:
- Line 76: Update the dump-ddl.sh entry in the bootstrap README to remove the
claim that connectors-ddl snapshots are regenerated automatically by the
workflow. Document that dump-ddl.sh must be run manually to regenerate the
committed DDL snapshot, and describe the workflow as a reminder rather than an
automatic regeneration step.
In `@src/ingestion/tests/e2e/lib/ch_seeder.py`:
- Around line 122-130: Update `_fetch_engine` and `_fetch_column_types` to avoid
interpolating `schema` and `table` directly into SQL, using the ClickHouse query
helper’s parameter-binding support or an identifier allowlist validation before
constructing the queries. Preserve the existing engine and column-type results
while removing the Ruff S608 findings without an inline suppression.
In `@src/ingestion/tests/e2e/lib/fixture_loader.py`:
- Line 125: Update load() to parse and validate the top-level skip declaration
immediately after loading the mapping, before validating cases or resolving
bronze schemas and fixture data. When skip is set, return a minimal TestYaml
carrying the skip value and omit normal fixture resolution; preserve existing
validation and loading behavior when no skip is declared.
- Line 125: Validate the value read by the fixture loader’s skip field before
assigning it to the annotated `str | None` property: accept only a string or
null, and reject booleans, lists, mappings, and other YAML types at the parsing
boundary. Preserve valid string skip values and existing absent-field behavior.
---
Nitpick comments:
In @.github/workflows/connectors-ddl-reminder.yml:
- Around line 6-9: Pin the actions/github-script usage in the workflow to a full
immutable commit SHA instead of the mutable `@v7` tag. Update only that action
reference and retain its existing configuration and behavior.
In `@src/ingestion/tests/e2e/lib/dbt_runner.py`:
- Around line 298-307: Remove allow_experimental_refreshable_materialized_view
from the settings mapping in dbt_runner.py, and remove any remaining SQL
settings that enable the same obsolete flag. Keep
allow_experimental_correlated_subqueries and all other required settings
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f229598c-4922-4cdd-81ae-974c697a6688
📒 Files selected for processing (89)
.claude/skills/metric-test/SKILL.md.github/workflows/connectors-ddl-reminder.ymlcharts/insight/templates/ingestion/dbt-run.yamldeploy/compose/clickhouse-user-defaults.xmldeploy/seed/silver.pydocker-compose.ymldocs/domain/ingestion/specs/ADR/0007-fresh-cluster-placeholders.mddocs/domain/ingestion/specs/DESIGN.mdsrc/ingestion/README.mdsrc/ingestion/connectors/ai/claude-admin/dbt/claude_admin__ai_dev_usage.sqlsrc/ingestion/connectors/ai/openai/dbt/to_ai_tool_usage.sqlsrc/ingestion/connectors/collaboration/m365/dbt/m365__collab_chat_activity.sqlsrc/ingestion/connectors/crm/salesforce/dbt/salesforce__crm_deals.sqlsrc/ingestion/connectors/git/github-v2/dbt/github__commits.sqlsrc/ingestion/connectors/git/github-v2/dbt/github__file_changes.sqlsrc/ingestion/connectors/git/github-v2/dbt/github__pull_requests_comments.sqlsrc/ingestion/connectors/git/github-v2/dbt/github__pull_requests_commits.sqlsrc/ingestion/connectors/git/github-v2/dbt/github__pull_requests_reviewers.sqlsrc/ingestion/connectors/git/github-v2/dbt/github__repositories.sqlsrc/ingestion/connectors/git/github-v2/dbt/schema.ymlsrc/ingestion/connectors/git/gitlab/dbt/gitlab__commits.sqlsrc/ingestion/connectors/git/gitlab/dbt/gitlab__pull_requests.sqlsrc/ingestion/connectors/task-tracking/jira/dbt/jira__task_comments.sqlsrc/ingestion/connectors/task-tracking/jira/dbt/jira__task_worklogs.sqlsrc/ingestion/dbt/.gitignoresrc/ingestion/dbt/identity/seed_aliases_from_claude_admin.sqlsrc/ingestion/dbt/identity/seed_persons_from_claude_admin.sqlsrc/ingestion/scripts/apply-ch-migrations.shsrc/ingestion/scripts/bootstrap-db/.env.bootstrap.examplesrc/ingestion/scripts/bootstrap-db/README.mdsrc/ingestion/scripts/bootstrap-db/bootstrap-db.shsrc/ingestion/scripts/bootstrap-db/connectors-config.yamlsrc/ingestion/scripts/bootstrap-db/create-connector-tables.shsrc/ingestion/scripts/bootstrap-db/dump-ddl.shsrc/ingestion/scripts/bootstrap-db/pins.envsrc/ingestion/scripts/bootstrap-db/run-dbt.shsrc/ingestion/scripts/bootstrap-db/seed-connectors.shsrc/ingestion/scripts/connectors-ddl/bamboohr.sqlsrc/ingestion/scripts/connectors-ddl/bitbucket-cloud.sqlsrc/ingestion/scripts/connectors-ddl/chatgpt-team.sqlsrc/ingestion/scripts/connectors-ddl/claude-admin.sqlsrc/ingestion/scripts/connectors-ddl/claude-enterprise.sqlsrc/ingestion/scripts/connectors-ddl/claude-team.sqlsrc/ingestion/scripts/connectors-ddl/confluence.sqlsrc/ingestion/scripts/connectors-ddl/cursor.sqlsrc/ingestion/scripts/connectors-ddl/figma.sqlsrc/ingestion/scripts/connectors-ddl/github-copilot.sqlsrc/ingestion/scripts/connectors-ddl/github-v2.sqlsrc/ingestion/scripts/connectors-ddl/gitlab.sqlsrc/ingestion/scripts/connectors-ddl/hubspot.sqlsrc/ingestion/scripts/connectors-ddl/identity.sqlsrc/ingestion/scripts/connectors-ddl/insight.sqlsrc/ingestion/scripts/connectors-ddl/jira.sqlsrc/ingestion/scripts/connectors-ddl/m365.sqlsrc/ingestion/scripts/connectors-ddl/ms-entra.sqlsrc/ingestion/scripts/connectors-ddl/openai.sqlsrc/ingestion/scripts/connectors-ddl/outline.sqlsrc/ingestion/scripts/connectors-ddl/person.sqlsrc/ingestion/scripts/connectors-ddl/salesforce.sqlsrc/ingestion/scripts/connectors-ddl/silver.sqlsrc/ingestion/scripts/connectors-ddl/slack.sqlsrc/ingestion/scripts/connectors-ddl/staging.sqlsrc/ingestion/scripts/connectors-ddl/workday.sqlsrc/ingestion/scripts/connectors-ddl/youtrack.sqlsrc/ingestion/scripts/connectors-ddl/zendesk.sqlsrc/ingestion/scripts/connectors-ddl/zoom.sqlsrc/ingestion/scripts/connectors-ddl/zulip-proxy.sqlsrc/ingestion/scripts/create-bronze-placeholders.shsrc/ingestion/silver/_shared/class_people.sqlsrc/ingestion/silver/_shared/identity_inputs.sqlsrc/ingestion/silver/hr/class_hr_events.sqlsrc/ingestion/silver/hr/class_hr_working_hours.sqlsrc/ingestion/tests/e2e/compose/docker-compose.ymlsrc/ingestion/tests/e2e/conftest.pysrc/ingestion/tests/e2e/lib/ch_seeder.pysrc/ingestion/tests/e2e/lib/clickhouse.pysrc/ingestion/tests/e2e/lib/dbt_runner.pysrc/ingestion/tests/e2e/lib/fixture_loader.pysrc/ingestion/tests/e2e/lib/migration_applier.pysrc/ingestion/tests/e2e/metrics/git_metrics.test.yamlsrc/ingestion/tests/e2e/metrics/schemas/bronze_confluence.wiki_footer_comments.yamlsrc/ingestion/tests/e2e/metrics/schemas/bronze_jira.jira_issue.yamlsrc/ingestion/tests/e2e/metrics/schemas/bronze_jira.jira_issue_history.yamlsrc/ingestion/tests/e2e/metrics/schemas/bronze_jira.jira_statuses.yamlsrc/ingestion/tests/e2e/metrics/schemas/bronze_jira.jira_worklogs.yamlsrc/ingestion/tests/e2e/metrics/schemas/bronze_m365.onedrive_activity.yamlsrc/ingestion/tests/e2e/metrics/schemas/bronze_m365.teams_activity.yamlsrc/ingestion/tests/e2e/metrics/tasks_worklog_accuracy.test.yamlsrc/ingestion/tests/e2e/metrics/templates/jira_task.yaml
💤 Files with no reviewable changes (1)
- src/ingestion/tests/e2e/metrics/templates/jira_task.yaml
Contributor
Author
|
Addressed the two nitpicks from the review summary as well:
|
Harden the connectors-ddl snapshot generator against silently producing an incomplete snapshot while still exiting 0: - dump-ddl.sh: capture the ClickHouse row lists into a variable and feed the loops via a here-string. A ch() (curl) failure inside a `done < <(ch ...)` process substitution is invisible to `set -e` (the parent only sees EOF), so a failed query silently emitted a truncated snapshot. Command substitution lets the non-zero exit abort. The grep-based staging loop is left as-is: its no-match exit 1 is benign and must not abort the dump. - bootstrap-db.sh: drop the `|| echo ... continuing` around run-dbt.sh. Step 4 sets SKIP_DBT_GOLD=1 (gold is built here, not there), so a swallowed dbt failure would ship an incomplete snapshot while bootstrap succeeds. A complete dbt run is a precondition for the dump. - create-connector-tables.sh: guard the missing dbt/ dir case so `find`'s non-zero exit under pipefail/set -e no longer aborts before the skip branch. Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
- fixture_loader: parse the top-level `skip` immediately after the mapping check and short-circuit to a minimal skipped TestYaml before validating cases or resolving/validating bronze. A blocked-metric fixture (e.g. git_metrics, whose data predates the github-v2 bronze schema) otherwise fails pytest collection instead of skipping. Also reject non-string `skip` at the YAML boundary so `skip: false` can no longer be silently ignored by the runner's truthiness check. - dbt_runner: drop the obsolete allow_experimental_refreshable_materialized_view profile setting; refreshable MVs are GA on ClickHouse 25.7.5 (the flag only emits an obsolete-setting warning). Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
- README: the snapshot is regenerated MANUALLY via dump-ddl.sh, not automatically; connectors-ddl-reminder.yml only posts a nudge. Correct the workflow name and the coverage description (person/identity/silver/insight plus gold-referenced staging). - metric-test SKILL: replace the obsolete 'add DDL to a bootstrap heredoc' steps for a new bronze table with the snapshot-regeneration workflow, since hand edits to the generated snapshot do not survive the next regeneration. Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
The reminder workflow runs on pull_request_target with pull-requests: write. Pin the action to an immutable commit SHA (v7.1.0) instead of the mutable @v7 tag so the referenced code cannot change underneath a write-capable job. Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
mitasovr
enabled auto-merge (squash)
July 24, 2026 11:07
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/ingestion/scripts/bootstrap-db/dump-ddl.sh (1)
80-95: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFail when a referenced relation is missing.
dump_relationcurrently logs and skips missing objects. For relations discovered frominsight.sqlandsilver.sql, this can produce a successful but incompletestaging.sql, causing fresh-cluster bootstrap to fail later when gold views reference the omitted relation. Return non-zero so snapshot generation aborts instead.🤖 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/dump-ddl.sh` around lines 80 - 95, Update dump_relation so a missing relation causes the function to return a non-zero status after reporting the error, instead of silently returning success. Ensure callers generating relations from insight.sql and silver.sql propagate that failure and abort snapshot generation rather than producing an incomplete staging.sql.src/ingestion/scripts/bootstrap-db/README.md (1)
14-20: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd the host-gateway mapping for Linux Docker Engine.
host.docker.internalis not provided automatically by Docker Engine on Linux; add--add-host=host.docker.internal:host-gatewayto this command, or document that this local setup requires Docker Desktop.🤖 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` around lines 14 - 20, Update the documented docker run command in the bootstrap database setup to include the host-gateway mapping for Linux Docker Engine, using --add-host=host.docker.internal:host-gateway. Keep the existing image, environment variables, ports, and container name unchanged.
🧹 Nitpick comments (1)
src/ingestion/scripts/bootstrap-db/dump-ddl.sh (1)
112-130: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake
staging.sqlidempotent.Unlike the other database snapshots, this block emits plain
CREATE DATABASE staging. Retrying snapshot application on a partially bootstrapped cluster will fail before creating the staging relations. UseCREATE DATABASE IF NOT EXISTS staging, or explicitly guarantee this snapshot is never reapplied.🤖 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/dump-ddl.sh` around lines 112 - 130, The staging snapshot must be safe to apply repeatedly. Update the database creation statement in the staging dump block before the relation loop to use an idempotent CREATE DATABASE IF NOT EXISTS form, preserving the existing staging relation dumping behavior.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@src/ingestion/scripts/bootstrap-db/dump-ddl.sh`:
- Around line 80-95: Update dump_relation so a missing relation causes the
function to return a non-zero status after reporting the error, instead of
silently returning success. Ensure callers generating relations from insight.sql
and silver.sql propagate that failure and abort snapshot generation rather than
producing an incomplete staging.sql.
In `@src/ingestion/scripts/bootstrap-db/README.md`:
- Around line 14-20: Update the documented docker run command in the bootstrap
database setup to include the host-gateway mapping for Linux Docker Engine,
using --add-host=host.docker.internal:host-gateway. Keep the existing image,
environment variables, ports, and container name unchanged.
---
Nitpick comments:
In `@src/ingestion/scripts/bootstrap-db/dump-ddl.sh`:
- Around line 112-130: The staging snapshot must be safe to apply repeatedly.
Update the database creation statement in the staging dump block before the
relation loop to use an idempotent CREATE DATABASE IF NOT EXISTS form,
preserving the existing staging relation dumping behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eedd70af-aba0-4ef9-8ffd-79100679e3e3
📒 Files selected for processing (10)
.claude/skills/metric-test/SKILL.md.github/workflows/connectors-ddl-reminder.ymlsrc/ingestion/connectors/task-tracking/jira/dbt/jira__task_comments.sqlsrc/ingestion/connectors/task-tracking/jira/dbt/jira__task_worklogs.sqlsrc/ingestion/scripts/bootstrap-db/README.mdsrc/ingestion/scripts/bootstrap-db/bootstrap-db.shsrc/ingestion/scripts/bootstrap-db/create-connector-tables.shsrc/ingestion/scripts/bootstrap-db/dump-ddl.shsrc/ingestion/tests/e2e/lib/dbt_runner.pysrc/ingestion/tests/e2e/lib/fixture_loader.py
💤 Files with no reviewable changes (1)
- src/ingestion/tests/e2e/lib/dbt_runner.py
🚧 Files skipped from review as they are similar to previous changes (6)
- src/ingestion/connectors/task-tracking/jira/dbt/jira__task_comments.sql
- src/ingestion/connectors/task-tracking/jira/dbt/jira__task_worklogs.sql
- .github/workflows/connectors-ddl-reminder.yml
- src/ingestion/scripts/bootstrap-db/create-connector-tables.sh
- .claude/skills/metric-test/SKILL.md
- src/ingestion/scripts/bootstrap-db/bootstrap-db.sh
The github-v2 rewrite stores commit parent_hashes as a JSON-array string in the Nullable(String) bronze column (Airbyte serializes the connector's array field), but github__commits.sql still detected merges with length() — which counts characters, not elements. A normal one-parent commit (`["sha"]`) is 6 chars, so it was wrongly flagged is_merge_commit=1, and the downstream non-merge filters (fct/mtr/gold) then dropped essentially every commit. Use JSONLength() to count array elements. Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
The fixture predated the github-v2 bronze rewrite and was skipped. Update it to the current schema so the git metrics run again: - commits: oid -> sha; parent_hashes array -> JSON-array string (matching how Airbyte lands the connector's array into the Nullable(String) column) - file_changes: commit_hash -> sha - schemas mirror the column/type renames - pull_requests were already github-v2-shaped (no change) Remove the top-level skip and its stale note. Metric values are unchanged (same additions/deletions per person). Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
mozhaev-dev
approved these changes
Jul 25, 2026
cyberantonz
added a commit
to cyberantonz/insight
that referenced
this pull request
Jul 27, 2026
…uctorfabric#1743) constructorfabric#1892 retyped jira__task_worklogs.worklog_id and jira__task_comments.comment_id projections to toString(), but incremental-append staging tables on existing stands keep the old Decimal(38,9) column; the positional union with the youtrack String twins then fails with NO_COMMON_TYPE and silver class_task_* stays empty. MODIFY the warm staging tables and their silver targets to Nullable(String) in the clickhouse-migrate heals (guarded, idempotent). Verified on CH 25.7.5.34: reproduced code 386, heal converges the union. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
mitasovr
pushed a commit
that referenced
this pull request
Jul 27, 2026
…#1940) #1892 retyped jira__task_worklogs.worklog_id and jira__task_comments.comment_id projections to toString(), but incremental-append staging tables on existing stands keep the old Decimal(38,9) column; the positional union with the youtrack String twins then fails with NO_COMMON_TYPE and silver class_task_* stays empty. MODIFY the warm staging tables and their silver targets to Nullable(String) in the clickhouse-migrate heals (guarded, idempotent). Verified on CH 25.7.5.34: reproduced code 386, heal converges the union. Signed-off-by: Anton Zelenov <antonz@constructor.tech> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 30, 2026
This was referenced Aug 7, 2026
3 tasks
3 tasks
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
2 participants
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.
What & why
Replaces the hand-written
create-bronze-placeholders.sh(ADR-0007) with agenerated DDL snapshot in
scripts/connectors-ddl/*.sql, dumped bybootstrap-dbfrom real connectors + dbt + migrations.create-bronze-placeholders.shis now a thin applicator of that snapshot, so a fresh cluster type-checks every
gold view without a hand-maintained placeholder list (#1831), and a fresh cluster
builds end-to-end (#1763). All environments are on ClickHouse 25.7.5 (test/compose
pins bumped to match).
Merges
upstream/main, including the bitbucket-cloud dbt fix (#1880).Generation pipeline / prod
person/identity(init-identity) are created before dbt so theidentity seed models'
ANTI JOIN person.personsresolves; gold-view migrations runafter dbt.
allow_experimental_correlated_subqueriesset at profile level in all four dbtprofile generators — a model
config()setting does not reach the SELECT plan.toUUID(...)(String vs UUIDNO_COMMON_TYPE).dump-ddlalso emitsperson/identityand only the gold-referenced stagingtables (today just
m365__collab_email_activity) — deliberately not all of staging:pre-creating an incremental staging model empty poisons its first
is_incrementalbuild (
max(date) FROM {{ this }}over an empty MergeTree reads the Date type-max on25.7 and excludes every row).
SKIP_DBT_GOLDlets generation skipapply-ch-migrations' own gold build (gold isalready built by the pinned dbt venv; avoids requiring
dbton PATH — absent /dbt-fusion outside the prod toolbox). Prod deploy leaves it unset.
apply-ch-migrationsheal section (create-bronze-placeholders is now the applicator).e2e rig (unified metrics)
_ensure_tableaccepts the real table being a superset of the fixture schema(connectors emit raw API columns the dbt models never read); seeder Decimal coercion +
jira fixture schema/value alignment to the real bronze.
skip:in*.test.yaml.Testing
PASS=204, ERROR=0, SKIP=0.git_metric_observations), allclass_*incl git,person.persons. Re-dump is byte-identical (converges); fresh-cluster apply validated.Follow-ups
git_metricse2e is skipped: its fixtures predate the github-v2 connector rewrite(
oid/commit_hash→sha, envelope columns). The git silver/gold pipeline itselfbuilds fine now that fix(git): de-shadow completed_at alias in Bitbucket staging models (ILLEGAL_AGGREGATION) #1880 is merged — only the fixtures need refreshing. Tracking
separately.
bootstrap-db.sh+dump-ddl.sh); thereis no auto-regenerate-and-commit CI job. Instead
.github/workflows/connectors-ddl-reminder.ymlposts a single sticky comment when a PR touches
src/ingestion/**, reminding the authorto regenerate and commit the snapshot. It uses
pull_request_target(so it also commentson fork PRs) but never checks out/runs PR code and uses no secrets, so it is safe; it
takes effect once merged to
main.mitasovrfork so the workflow file could be included (a same-repo OAuthpush rejects
.github/workflows/**withoutworkflowscope). Supersedes feat(ingestion): bootstrap-db — generate connectors-ddl snapshot [WIP] #1876.Summary by CodeRabbit