aminghadersohi commented on PR #40344:
URL: https://github.com/apache/superset/pull/40344#issuecomment-4565824882
A few schema issues worth addressing before merge:
**`TaskInfo` missing `started_at`, `ended_at`, `user_id`**
The Task model has `started_at` and `ended_at` (DateTime) and `user_id`
(Integer). Without these, `list_tasks` / `get_task_info` can't answer the most
basic monitoring questions: when did this task run, and who triggered it.
Recommend adding all three to `TaskInfo`, `ALL_TASK_COLUMNS`, and
`serialize_task_object`.
**`TaskColumnFilter` should allow `started_at`, `ended_at`, `user_id`**
No time-range filtering on `list_tasks` makes it much less useful for
operational monitoring. Recommend adding these to `col: Literal[...]`.
**Naming inconsistency: `TaskColumnFilter` and `RlsColumnFilter` (in
#40347)**
Every other filter class in the MCP suite follows the pattern
`<Resource>Filter` (`QueryFilter`, `TagFilter`, `RoleFilter`, `ReportFilter`,
etc.). `TaskColumnFilter` and `RlsColumnFilter` are the only outliers. Suggest
renaming to `TaskFilter` and `RlsFilter` respectively for consistency.
**`ActionLogInfo` missing `duration_ms` and `referrer`**
The `Log` model has `duration_ms = Column(Integer)` and `referrer =
Column(String(1024))`. `duration_ms` is particularly valuable for performance
analysis ("show me slow actions"). Recommend adding both to the schema and
`ALL_LOG_COLUMNS`.
--
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]
