7487 commented on PR #4018:
URL: https://github.com/apache/iggy/pull/4018#issuecomment-5539133396
Thanks for the thorough pass — all addressed, point by point:
- **`CacheMetricsKey` constructor**: added `#[new]`, so
`CacheMetricsKey(stream_id=1, topic_id=2, partition_id=3)` now works, and there
is a new test that an independently constructed equal key addresses a dict
entry (eq/hash round-trip).
- **`cache_metrics` re-collection**: the map is now converted exactly once
in `From<RustStats>` and stored as a `Py<PyDict>`; the getter hands back the
same dict on every access, so iterate-and-index is linear. The test asserts
`stats.cache_metrics is stats.cache_metrics` to pin that down.
- **Non-empty assertion**: I could not make that work, and traced why — the
server currently hardcodes an empty map in the `GetStats` reply
(`core/server/src/responses.rs`: `cache_metrics: Vec::new()`), so the map stays
empty no matter how much traffic precedes the call. A non-empty assertion would
fail unconditionally. I left the per-entry type checks (with a comment
explaining they only run once the server starts populating the field) and
covered the key/dict semantics in the new server-free test instead.
- **Vacuous assertions** (`is not None`, `>= 0` on unsigned): dropped.
- **Exact equality on server-global counters**: switched to `>=` with a
comment naming the pytest-xdist reason.
- **Semver docstring**: you're right — `get_numeric_version` formats
`{major}{minor:03}{patch:03}`, so 1.2.3 -> 1002003. Fixed in the binding, in
`core/common/src/types/stats/mod.rs`, and the stub is regenerated.
Also merged master to catch the branch up.
--
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]
