Skip to content

fix(examples/playground): use canonical target_dialect in portability POC#201

Merged
hugocorreia90 merged 1 commit intomainfrom
fix/poc-portability-target-dialect
Apr 21, 2026
Merged

fix(examples/playground): use canonical target_dialect in portability POC#201
hugocorreia90 merged 1 commit intomainfrom
fix/poc-portability-target-dialect

Conversation

@hugocorreia90
Copy link
Copy Markdown
Contributor

Summary

examples/playground/pocs/06-developer-experience/08-portability-lint/rocky.toml had target_dialect = "bq", which is invalid both at the TOML schema level and at runtime — rocky_sql::transpile::Dialect uses #[serde(rename_all = "lowercase")] and only accepts databricks / snowflake / bigquery / duckdb in [portability] blocks. The short-form bq / sf / dbx are CLI-flag ergonomics only (rocky compile --target-dialect bq), converted to the long form at the clap boundary.

Flipping one TOML line to "bigquery" unblocks every_committed_poc_matches_project_schema on main, which has been red since the POC landed in #196.

Why POC behavior is unchanged

run.sh already overrides the TOML with --target-dialect bq on the CLI — that short form stays valid at the flag level. The TOML value is only consulted when no CLI override is present, so the POC's demonstration flow is identical.

Test plan

  • cargo test -p rocky-core --test project_schema every_committed_poc_matches_project_schema — now passes
  • Confirmed on main (pre-fix): test fails with "bq" is not valid under any of the schemas listed in the 'anyOf' keyword at /portability/target_dialect
  • POC smoke: ./run.sh still exits 0 in the 08-portability-lint POC (reviewer sanity-check)

Context: drift caught by the parallel agent while implementing rocky branch compare (PR #200).

… POC

The 08-portability-lint POC had `target_dialect = "bq"` in rocky.toml,
but rocky-sql's Dialect enum uses `#[serde(rename_all = "lowercase")]`
which only accepts `databricks`/`snowflake`/`bigquery`/`duckdb` in
TOML. The short-form `bq`/`sf`/`dbx` are CLI-flag ergonomics only (see
transpile.rs comment).

POC runtime behavior is unchanged — run.sh overrides via
`rocky compile --target-dialect bq` which accepts short forms at the
CLI boundary.

Fixes `every_committed_poc_matches_project_schema` failing on main.
@hugocorreia90 hugocorreia90 merged commit f50e4b8 into main Apr 21, 2026
7 checks passed
@hugocorreia90 hugocorreia90 deleted the fix/poc-portability-target-dialect branch April 21, 2026 14:02
@hugocorreia90 hugocorreia90 mentioned this pull request Apr 21, 2026
7 tasks
hugocorreia90 added a commit that referenced this pull request Apr 21, 2026
* chore(engine): release 1.12.0

Arc 1 wave 2 + cleanup cascade. Eight PRs since v1.11.0:

- #199 SIGPIPE handler
- #200 rocky branch compare
- #201 POC target_dialect cleanup
- #202 rocky cost <run_id|latest> (Arc 2 wave 2 first PR)
- #203 rocky run persists RunRecord (Arc 1 wave 2 load-bearing fix)
- #204 docs + CHANGELOG [Unreleased] cascade
- #205 demo-branches-replay.gif refresh
- #206 real per-model started_at on MaterializationOutput

rocky history / replay / trace / cost now return real data end-to-end
for the first time. Full notes in CHANGELOG.

* feat(state): configurable transfer timeout + tracing span + Valkey wrap

- `StateConfig.transfer_timeout_seconds` (default 300s) replaces the hard-
  coded `STATE_TRANSFER_TIMEOUT`. Operators can now tune the wall-clock
  budget in `rocky.toml` for very large state or slow networks without
  recompiling. `StateConfig` gains a manual `Default` impl so
  `StateConfig::default()` yields 300s (not u64's zero).
- `state.upload` / `state.download` tracing spans wrap every transfer
  carrying `backend`, `bucket`, and `size_bytes`. The in-elapse warn event
  inherits those fields automatically, so hung transfers are diagnosable
  from stderr logs alone (which dagster-rocky streams into the Dagster run
  viewer).
- Structured `warn!` on timeout elapse ("state transfer exceeded timeout
  budget") with a `duration_ms` field — replaces silent `Timeout(_)`.
- Valkey read/write paths audited and closed: `redis::Client::get_connection`
  + `redis::cmd(...).query()` are sync and blocked the tokio runtime thread;
  no outer `tokio::time::timeout` could rescue them. Both `upload_to_valkey`
  and `download_from_valkey` now run under `tokio::task::spawn_blocking`
  inside `with_transfer_timeout`, closing the same class of hang the
  object-store paths were already protected against.
- `default_client_options()` in `object_store.rs` honours the standard
  `object_store`-crate env vars `AWS_ALLOW_HTTP` / `AZURE_ALLOW_HTTP` /
  `GOOGLE_STORAGE_ALLOW_HTTP`. Always off in production; the new
  integration test uses it to front-end the S3 SDK with a plain-HTTP
  wiremock server without bypassing the credential chain.
- New `tests/state_sync_timeout_test.rs` integration test: a wiremock S3
  endpoint that holds PutObject for 1h proves `upload_state` returns
  `StateSyncError::Timeout` within the configured 2s budget (+grace).
  A prompt-endpoint negative control guards against regressions.
- CHANGELOG entries added under [1.12.0]. Example config in
  `engine/examples/dagster-integration/rocky.toml` surfaces the new key.

cargo fmt clean; `clippy --workspace --all-targets -- -D warnings` clean;
all 977 rocky-core unit tests + 30 e2e + 20 integration + the 2 new
timeout tests pass.

* chore(codegen): regenerate schemas + pydantic types for StateConfig.transfer_timeout_seconds

* chore(fixtures): regenerate dagster test fixtures for 1.12.0

`just regen-fixtures` — version string bump only (1.11.0 → 1.12.0) across
35 captured fixtures under integrations/dagster/tests/fixtures_generated/.
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.

1 participant