mikebridge commented on PR #39977: URL: https://github.com/apache/superset/pull/39977#issuecomment-4432696327
After some discussion with AI about the maintainability of the official
side-effecting pattern, I'd like to create a follow-up PR after this one to
wrap the per-request bypass in a context manager.
The issue here is that it's easy for a dev to forget to reset the flag in
try/catch:
- Forgetting the `try/finally` and leaving the flag set for the rest of
the request
- Cleaning up with `setattr(g, SKIP_VISIBILITY_FILTER, False)` instead of
restoring the captured previous value, which breaks composition when a caller
is already inside a bypass
Both cases can be eliminated by a small context manager next to the
`SKIP_VISIBILITY_FILTER` constant (via claude):
```python
@contextmanager
def skip_visibility_filter() -> Iterator[None]:
"""Opt the current request out of the soft-delete listener for the
duration of the block. Restores the previous value on exit,
including the exception path. Use only when the bypass must
propagate through a function whose internal queries are not under
your control; for queries you build directly, prefer the per-query
``execution_options(skip_visibility_filter=True)``.
"""
previous = getattr(g, SKIP_VISIBILITY_FILTER, False)
setattr(g, SKIP_VISIBILITY_FILTER, True)
try:
yield
finally:
setattr(g, SKIP_VISIBILITY_FILTER, previous)
```
Call site in BaseRestoreCommand.validate becomes:
```python
with skip_visibility_filter():
try:
security_manager.raise_for_ownership(model)
except SupersetSecurityException as ex:
raise self.forbidden_exc() from ex
```
--
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]
