bito-code-review[bot] commented on code in PR #44386:
URL: https://github.com/apache/superset/pull/44386#discussion_r4052835133
##########
superset/mcp_service/middleware.py:
##########
@@ -71,6 +76,40 @@
# tools) while rejecting StatsD metadata characters and unbounded lengths.
_METRIC_TOOL_NAME_RE = re.compile(r"[A-Za-z0-9_][A-Za-z0-9_.\-]{0,127}")
+# Character cap for the free-form string fields kept in a minimal
committed-write
+# confirmation (see ``_shrink_minimal_response``). Generous enough to keep a
+# chart name or a short error readable, small enough that the whole
confirmation
+# stays bounded no matter how large the fields were in the original payload.
+_MINIMAL_FIELD_CHARS = 200
+
+# Identifying fields kept when a structured ``error`` has to be reduced to fit
+# (see ``_clip_error``). Everything else on ``MCPBaseError`` and its subclasses
+# is an unbounded container -- ``validation_errors``, ``dataset_context``,
+# ``query_info``, ``suggestions`` -- any of which can dwarf the write
+# confirmation it is riding on. ``error`` mirrors ``message`` as a
+# backward-compatible alias, so both are kept.
+_MINIMAL_ERROR_FIELDS = ("error_type", "error", "message", "error_code",
"details")
+
+# Scalar fields kept when a committed write's identifying object (a nested
+# ``chart``/``dashboard``/``metric`` dict) has to be reduced to fit. These are
+# the names across the info models that answer "what was written" --
+# everything else on them is either unbounded or irrelevant to that question.
+# ``is_unsaved_state`` is here because update_chart defaults to
+# ``generate_preview=True`` and then persists nothing, so it is the caller's
+# only in-band way to tell a cached preview from a persisted write.
+_MINIMAL_IDENTITY_FIELDS = (
+ "id",
+ "uuid",
+ "url",
+ "slice_name",
+ "dashboard_title",
+ "metric_name",
+ "table_name",
+ "dataset_name",
+ "label",
+ "is_unsaved_state",
+)
Review Comment:
<!-- Bito Reply -->
The reviewer's suggestion is not applicable here because the restore and
delete tools use top-level scalar identity fields (`restored_id`, `deleted_id`,
etc.). Since these fields are not nested within the identifying objects that
`_shrink_minimal_response` reduces, they are preserved by the scalar pass and
do not require inclusion in `_MINIMAL_IDENTITY_FIELDS`.
--
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]
