aminghadersohi commented on PR #35591: URL: https://github.com/apache/superset/pull/35591#issuecomment-3498910424
I've addressed the reviewer feedback with the latest commit. Here's what changed: ## Changes Made ### 1. Reverted Exception Signature Changes - Removed the `__init__` method overrides from timeout exception classes - Restored the original class attribute `message` pattern - All user-facing messages remain wrapped in `_()` for proper translation ### 2. Separated User-Facing from Internal Messages - User-facing messages: Use the translated default messages from exception classes - Internal diagnostic info: Moved to `logger.error()` calls with chart_id, execution_id, and timeout values - This follows the principle that exception messages should be user-friendly while detailed context goes to logs ### 3. Updated Tests - Test now expects the default translated message instead of the detailed error string - This aligns with the new approach where detailed info is logged separately ## Addressing Reviewer Concerns **@eschutho**: Translation functions (`_()`) are now properly used for all user-facing error messages **@sadpandajoe**: No need for `lazy_gettext()` since we're using class attribute messages (evaluated at import time) **@dpgaspar**: Exception signature remains unchanged from the original implementation - just using class attributes The key insight is that these error messages appear in execution logs and notifications to users, so they should be translated. The detailed diagnostic information (chart_id, timeout, etc.) is now logged separately for debugging purposes. -- 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]
