gabotorresruiz commented on PR #44396:
URL: https://github.com/apache/superset/pull/44396#issuecomment-5780964843
Re-checked at `e2884361`. Confirming the approval still stands after the
master merge.
The branch tip did not move: `e2884361^1` is `ae8e8cb9`, the commit I
approved, so this is a master merge only. I generated this PR's own patch at
both points (`43fee87..ae8e8cb9` and `cf39d22..e2884361`) and diffed them:
identical line for line, 1666 lines each. The only differences are neighbouring
`UPDATING.md` entries and hunk offsets in `superset/charts/api.py` and
`superset/dashboards/api.py`. The export and import format did not move, and
every file that determines the two compatibility directions is unchanged on
master as well, including `superset/charts/schemas.py`, both export commands,
all three importers and `superset/commands/dashboard/importers/v1/utils.py`.
I re-ran the round trip on this head anyway, because master merged a rewrite
of `set_related_perm` (`superset/models/slice.py:495`), a `before_insert`
listener that fires on the exact `Slice` insert the importer performs. Export
from a metastore with `SemanticView(id=7)` and an unrelated `SqlaTable(id=7)`,
import into a separate empty metastore where the same uuid lives at `id=81`
with tables squatting on `81` and `7`: the chart still lands on
`81__semantic_view` across `params` and `query_context`, both semantic targets
rebind to `81`, the table targets rebind to the newly imported dataset, and the
rewritten listener copies the destination view's `perm` onto the chart rather
than the colliding table's. Five modules from the description are 128 passed
here, and `tests/unit_tests/{commands,semantic_layers,charts,dashboards}` is
2549 passed with 26 skipped and 2 xfailed.
One thing worth knowing: against current master this no longer merges. `git
merge-tree` gives exactly one conflict, at `superset/dashboards/api.py:153`,
where this branch adds `from superset.semantic_layers.import_export import
SemanticReferenceError` and master adds `from superset.semantic_layers.models
import SemanticView` on the same line. Keeping both resolves it, and it does
not touch the bundle format.
My two earlier comments are still open. Neither is blocking.
--
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]
