DevShiba commented on PR #24218: URL: https://github.com/apache/datafusion/pull/24218#issuecomment-5394694234
@kosiew Done — reverted the `BytesCount`/`BytesGauge` enum-variant approach entirely and restored the original category-aware `Display for Metric` design, per your review: - `MetricValue` is back to its exact pre-PR shape (`Count`/`Gauge`/`PeakMemoryUsage` only, no new variants) — no `enum_variant_added` SemVer break. - `bytes_counter`/`bytes_gauge`/`global_bytes_counter` in `MetricBuilder` still tag the metric with `MetricCategory::Bytes`, but now construct generic `Count`/`Gauge` instead of dedicated variants. - `Display for Metric` checks `metric_category == Some(MetricCategory::Bytes)` and renders through `human_readable_size` for `Count`/`Gauge` in that case, falling back to `Display for MetricValue` otherwise (which is unchanged from before this PR touched it). - `FFI_MetricValue` in `datafusion-ffi` is reverted to its exact original shape — no new variants, no ABI change. - The cross-library FFI integration test (`test_ffi_execution_plan_byte_metrics_cross_library`) now asserts the transported `Bytes` category, the generic `Count`/`Gauge` variant/name/value, and the byte-formatted `Display` output separately, instead of matching on a dedicated variant discriminant — confirms the category survives the FFI round-trip correctly. - Updated `test_bytes_counter_and_gauge_use_byte_units` to assert the new (and correct) invariant: any `Count`/`Gauge` tagged `MetricCategory::Bytes` is byte-formatted, and one with no category (or a different category) is not. Verified: `cargo fmt --check`, the full `ci/scripts/rust_clippy.sh` (`-D warnings`), the `datafusion-ffi --features integration-tests` suite (including the rewritten cross-library test), and all three CI test-job commands from `rust.yml`/`extended.yml` (workspace, `force_hash_collisions`, and `extended_tests` feature sets) all pass clean against the current committed state, merged up to date with `main`. -- 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]
