sadpandajoe commented on code in PR #43458:
URL: https://github.com/apache/superset/pull/43458#discussion_r3978550891
##########
superset/security/manager.py:
##########
@@ -2512,8 +2527,10 @@ def get_datasource_access_error_object( # pylint:
disable=invalid-name
level=ErrorLevel.WARNING,
extra={
"link": self.get_datasource_access_link(datasource),
+ # is_access_denial lets the frontend show the "Request access"
+ # UI without receiving the dataset name.
+ "is_access_denial": True,
Review Comment:
A browser still running the previous frontend bundle does not recognize this
new payload because its access-denial guard requires a truthy
`extra.datasource_name`, so it falls back to “Unexpected error” and drops the
owners and Request access guidance. Could we keep a non-sensitive legacy
discriminator or explicitly stage the rollout so the reverse mixed-version
pairing remains functional?
##########
superset-frontend/src/components/ErrorMessage/DatasourceSecurityAccessErrorMessage.tsx:
##########
@@ -55,7 +56,14 @@ export function DatasourceSecurityAccessErrorMessage({
// (e.g. virtual-dataset SQL validation: "Only SELECT statements are
// allowed"). Those errors carry no access payload — render them plainly
// rather than misleading the user with request-access guidance.
- const isAccessDenial = !!extra?.datasource_name || !!extra?.tables?.length;
+ //
+ // `datasource` is the pre-`is_access_denial` shape of the payload; accepting
+ // it keeps this guidance working while a rolling deploy still has older API
+ // pods answering. It never appears on the non-access failures above.
+ const isAccessDenial =
+ !!extra?.tables?.length ||
+ !!extra?.is_access_denial ||
+ isDefined(extra?.datasource);
Review Comment:
During a rolling deploy, an older API pod can already expand
`{datasource_name}` into `extra.link`; this compatibility path deletes only the
sibling field, so the denied name is still exposed in the rendered Request
access URL. Could we suppress or sanitize that legacy link before storing or
rendering the error?
--
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]
