korbit-ai[bot] commented on code in PR #31798:
URL: https://github.com/apache/superset/pull/31798#discussion_r1911862463
##########
superset/views/base_api.py:
##########
@@ -130,6 +131,29 @@ def wraps(self: BaseSupersetApiMixin, *args: Any,
**kwargs: Any) -> Response:
return functools.update_wrapper(wraps, f)
+def validate_feature_flags(
+ feature_flags: list[str],
+) -> Callable[[Callable[..., Response]], Callable[..., Response]]:
+ """
+ A decorator to check if all given feature flags are enabled.
+
+ :param feature_flags: List of feature flag names to be checked.
+ """
+
+ def decorate(f: Callable[..., Response]) -> Callable[..., Response]:
+ @functools.wraps(f)
+ def wrapper(
+ self: BaseSupersetModelRestApi, *args: Any, **kwargs: Any
+ ) -> Response:
+ if not all(is_feature_enabled(flag) for flag in feature_flags):
+ return self.response_404()
Review Comment:
### Incorrect HTTP Status Code for Disabled Features <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The decorator returns a 404 Not Found response when feature flags are
disabled, which is semantically incorrect and misleading.
###### Why this matters
A 404 response indicates a resource was not found, but in this case, the
resource exists but is intentionally disabled. This could confuse API clients
and monitoring systems trying to understand why the endpoint is not accessible.
###### Suggested change ∙ *Feature Preview*
Return a 403 Forbidden status code which better represents that the feature
exists but access is not permitted:
```python
if not all(is_feature_enabled(flag) for flag in feature_flags):
return self.response_403()
```
</details>
###### Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help
Korbit improve your reviews.
<!--- korbi internal id:32607a73-c02f-4dc9-aa22-d71aa35ca40b -->
--
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]
