aminghadersohi commented on PR #44284:
URL: https://github.com/apache/superset/pull/44284#issuecomment-5687518205
Ownership update for `144a0df5e0075905aeabcbb449a82f761922b6dc` (rebased
onto master `a184395eca7c291d0b8b438f00690c633112f4d7`).
### What changed in this push
1. **Rebase.** Clean rebase onto current master, no conflicts. The diff is
still exactly three files: `superset/jinja_context.py` (+5),
`tests/unit_tests/jinja_context_test.py`, and
`docs/docs/using-superset/sql-templating.mdx`. `git rev-parse
HEAD:superset/migrations` equals `master:superset/migrations`
(`8e763047ee8802b1dca0339be4dc0897279def46`) — this PR does not touch
migrations.
2. **Addressed the bot suggestion on `test_where_in_literal_percent`.** The
before/after pair was never tautological — `where_in` is invoked between the
two compilations, so the pair is what proves the macro does not unescape the
shared dialect globally. But the assertion compared against an opaque value,
which is a fair thing to object to. It now pins both compilations to an
**explicitly computed** literal derived from the `paramstyle` rather than read
back off the dialect:
```python
compiler_escaped = (
escaped.replace("%", "%%") if paramstyle in {"format", "pyformat"}
else escaped
)
assert str(bind.compile(...)) == f"'{compiler_escaped}'"
assert where_in([value]) == f"('{escaped}')"
# macro ran between the two compilations
assert str(bind.compile(...)) == f"'{compiler_escaped}'"
```
Both assertions are mutation-tested. Deleting the fix from
`jinja_context.py` fails **35** cases; making the macro mutate the dialect
(`_double_percents = False` after unescaping) fails **18** cases. Full module:
**202 passed**, including `test_where_in_athena_parameterless_execution`, which
genuinely runs here (PyAthena 2.25.0 installed, not skipped). All changed-file
pre-commit hooks pass (mypy, ruff, ruff-format, pylint).
### CI: 46 pass, 12 skipped-as-inapplicable, 15 fail — all 15 from one
master defect
Every required gate that can run is green, including **`unit-tests
(current)`**, **`unit-tests-required`**, **`pre-commit (current)`**,
`lint-check`, `enforce-single-migration-head`, `codecov/patch`, `Analyze
(python)`, and `docker-build (superset)` / `docker-build (lean)`. The earlier
`test_stop_racing_normal_completion_...` flake did not recur.
The 15 failures are **12 execution jobs plus 3 aggregator gates**, and I
opened the log of each of the 12 individually. All 12 fail at database
initialization, before any test in this PR runs, with the identical error:
```
ERROR [flask_migrate] Error: Multiple head revisions are present for given
argument 'head'
```
`test-postgres (current)`, `test-mysql`, `test-sqlite`,
`test-load-examples`, `test-postgres-hive`, `test-postgres-presto`,
`cypress-matrix (chrome)`, `playwright-tests (chromium)`, `playwright-tests
(chromium, /app/prefix)`, `playwright-tests-experimental (chromium)`,
`playwright-tests-experimental (chromium, /app/prefix)`, `docker-build (dev)`.
`test-postgres-required`, `playwright-tests-required`, and
`cypress-matrix-required` are aggregators with no failure of their own.
This is a master defect, not a defect in this PR:
- Parsing every revision under `superset/migrations/versions` at master
`a184395` yields **two heads**: `c7f53d184ea2` (coordinate purge audit pruning)
and `88a01c781622` (index `ab_user(lower(username))`).
- The migration tree on this branch is byte-identical to master's, as shown
above.
- Unrelated, frontend-only open PRs on the same base show the same
`docker-build (dev)` and `playwright-tests (chromium)` failures — e.g. #44313
and #44308.
I am not adding a migration repair to this PR, and I am not re-running
deterministically blocked jobs. **This is explicitly not an all-green claim.**
### The one human action needed
A committer needs to merge a merge-head migration to master. #44288 already
does exactly that — it adds `e2f3a1b9c640` with `down_revision =
("c7f53d184ea2", "88a01c781622")`, no schema or data change — and **its own CI
is fully green (65 pass, 16 skipped, 0 fail)**, which independently confirms
the double head is the sole blocker. It is `MERGEABLE` and waiting on
`REVIEW_REQUIRED`.
Once #44288 (or an equivalent repair) lands, I will rebase this PR onto it
and re-run CI here.
Review still requested from @rusackas. No approval claimed; PR remains open
and non-draft.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
