Abdulrehman-PIAIC80387 commented on PR #42793:
URL: https://github.com/apache/superset/pull/42793#issuecomment-5216479833

   Thanks for the deep read @aminghadersohi. Pushed `123746ca1e` addressing all 
four findings plus a self-noticed weakness in the other two tests:
   
   1. **Rebased on master** to clear the `CONFLICTING` state.
   2. **UPDATING.md** now shows the sha256-default digest 
(`create_time_b09621`, not the md5 `b16a62`) and calls out cache invalidation + 
downstream consumers alongside CSV/XLSX headers.
   3. **Test #1** now uses fixed sha256 literals + patched 
`get_hash_algorithm`; tests #2 and #3 also hardened with `startswith(label + 
"_")` checks so they no longer pass against an identity `_mutate_label` (per 
your own observation).
   4. **PR body / commit message** reframed from "restore" to "add … widened to 
both drivers" per your point about the leaf-vs-base placement — Code 215 is 
server-side, so both `ClickHouseEngineSpec` and `ClickHouseConnectEngineSpec` 
need coverage.
   
   Also proactively added a **"why not `prefer_column_name_to_alias=1`"** 
section in the PR body, since @villebro floated that alternative on the [#38280 
thread](https://github.com/apache/superset/pull/38280#issuecomment-3103856611). 
TL;DR: both are valid; this PR takes the client-side path because it covers the 
sqlalchemy driver too and doesn't require touching existing `connect_args`. 
Happy to layer the setting default as a follow-up if maintainers prefer 
belt-and-braces.
   
   Note on CI: the failing checks are a master-wide alembic branching issue 
(`Multiple head revisions are present`), not this PR — every open PR is hitting 
it. Will re-run once master lands the merge.


-- 
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]

Reply via email to