Skip to content

Read the warehouse catalog and schema from settings - #4005

Open
blarghmatey wants to merge 2 commits into
mainfrom
tmacey/warehouse-catalog-setting
Open

blarghmatey wants to merge 2 commits into
mainfrom
tmacey/warehouse-catalog-setting

Conversation

@blarghmatey

Copy link
Copy Markdown
Member

What are the relevant tickets?

Follow-up to mitodl/ol-infrastructure#5719, which left mit-learn's QA StarRocks settings unset because of this.

Description (What does it do?)

BaseWarehouseETLTask subclasses declare their view as a literal ol_data_lake_production.ol_warehouse_production_integrations.<view>, with no setting to override it. RC can't run a warehouse-pull task against QA data as a result, so none of the Cohort 1 catalog sources (mitxonline, xpro, mit_edx, ocw, oll) can be rehearsed before cutover.

This adds WAREHOUSE_CATALOG and WAREHOUSE_SCHEMA settings, defaulting to the production pair. Tasks now declare a bare table_name, and view_name becomes a property that joins the three. SyncProgramCertificatesTask is the only subclass and resolves to the same view as before, so production is unchanged.

Each part is checked against [A-Za-z0-9_]+ separately, because a dotted value (e.g. WAREHOUSE_CATALOG=ol_data_lake_qa.x) would still pass iter_rows's check once joined but point somewhere else. iter_rows keeps its own check on the composed name.

A subclass that still assigns view_name would shadow the property and skip the settings silently, so __init_subclass__ raises TypeError for that. #3565 adds one such test task in warehouse_test.py, which will need table_name = "integrations__learn__test" after rebasing onto this.

The production default is deliberate (it keeps production's config untouched), and it means a non-production deploy that sets STARROCKS_HOST without the two new settings reads production. QA's ol-infrastructure change should set all four together. It also has to wait until QA actually has the views: ol_warehouse_qa_integrations has no tables today (Glue, 2026-09-28), vs. 11 integrations__learn__* tables in production.

Screenshots (if appropriate):

N/A, backend only.

How can this be tested?

  • learning_resources/lib/ and profiles/tasks_test.py against scratch Postgres/Redis containers: 50 passed, 3 skipped. The skips are the live-StarRocks tests in warehouse_integration_test.py, which need STARROCKS_HOST and run in CI's StarRocks service container. test_base_warehouse_etl_task_runs_against_real_starrocks now goes through the composed name (default_catalog.<scratch db>.<table>), which I haven't run locally.
  • New unit tests cover the production default, a QA override, rejection of dotted/unsafe/empty parts (settings and table_name) before any connection opens, and the view_name subclass guard.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Fa1mSBw5fudATLND9pAqTM

blarghmatey and others added 2 commits September 28, 2026 16:12
Every BaseWarehouseETLTask pinned its view to
ol_data_lake_production.ol_warehouse_production_integrations, so RC had no
way to run a warehouse-pull task against QA data. Tasks now declare a bare
table_name and view_name composes it with WAREHOUSE_CATALOG and
WAREHOUSE_SCHEMA, which default to the production pair.

Each part is checked on its own because a dotted setting would still pass
iter_rows's check once joined, but address a different catalog.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fa1mSBw5fudATLND9pAqTM
A subclass assigning view_name as a class attribute shadows the property and
skips WAREHOUSE_CATALOG/WAREHOUSE_SCHEMA without any error, which is the
pattern the open ownership-guard PR and the closed Cohort 1 task PR both use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fa1mSBw5fudATLND9pAqTM
@blarghmatey
blarghmatey requested a review from a team as a code owner September 28, 2026 20:18
Copilot AI balanced review requested due to automatic review settings September 28, 2026 20:18
@github-actions

Copy link
Copy Markdown

OpenAPI Changes

No changes detected

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@shanbady
shanbady self-requested a review September 29, 2026 13:29

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants