villebro commented on PR #44508:
URL: https://github.com/apache/superset/pull/44508#issuecomment-5778908207

   Pushed b950a5fbe5 on top of Amin's calibration fix (d784db26e7). Two things 
his fix didn't cover:
   
   1. **Stale "100k"/"100 KB" references it missed**: the actual user-facing 
`ValueError` in `validate_embedded_dimensions_page_size` still said "~100k by 
default" (now "~50k"), and `string_clip_chars`'s docstring still referenced 
"the default 100 KB budget" (now 50 KB, plus its inline `4` divisor is now a 
named `_CLIP_BUDGET_DIVISOR` constant).
   
   2. **`EMBEDDED_DIMENSIONS_MAX_PAGE_SIZE` was still 8**, which was only ever 
safe under the *old* 100,000-byte default. Measured directly against this 
repo's own `MetricInfo`/`DimensionInfo` schemas (40 dims/metric, matching the 
`large_metric_catalog` fixture): page_size=8 serializes to 54,558 bytes with no 
descriptions and 77,014 with short descriptions — both already over the new 
50,000 default. Since `list_metrics` isn't in the response-size guard's 
truncation-eligible tool categories, an oversized response there hard-errors 
rather than degrading, so this wasn't just a documentation gap. Lowered the cap 
to 4 (38,555 bytes / ~22% margin in the with-descriptions case) and updated 
every embedded-dimensions boundary test accordingly.
   
   Also addressed Bito's 3 non-blocking suggestions from the earlier bot 
review: folded the duplicate 
`test_small_budget_scales_string_clip_to_fit`/`test_truncates_large_string_fields`
 tests, named the clip-budget divisor constant, and changed 
`format_size_limit_error`'s sentinel check from `>=` to `==` since 
`UNMEASURABLE_RESPONSE_BYTES` is the only failure value.


-- 
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]

Reply via email to