Skip to content

Fix stats dump interval validation - #109

Merged
pniedzielski merged 2 commits into
bloomberg:mainfrom
mylee04:fix/stats-dump-interval-validation
Oct 19, 2026
Merged

pniedzielski merged 2 commits into
bloomberg:mainfrom
mylee04:fix/stats-dump-interval-validation

Conversation

@mylee04

@mylee04 mylee04 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Validate stats_dump_interval as a multiple of 30 seconds below 60 minutes before passing it to the native client. This raises ValueError for values such as 3600 and 31 seconds instead of triggering a C++ assertion that terminates the Python process.

Update the documented range to [0s, 60min) and add regression coverage for rejected values and the largest valid value, 3570 seconds.

Closes: #108

Testing

  • tests/unit/test_session.py: 80 passed on Python 3.12/macOS arm64 using the source Python package with the published 1.2.2 native extension.
  • Both Session and Session.with_options reject invalid values before native construction; None and valid values (including 0 and 3570 seconds) are forwarded unchanged to the native constructor.
  • All 80 session unit tests also passed on Ubuntu 24.04/Linux arm64 with a locally rebuilt native extension. Native mock controls through both public APIs confirmed the default, 30 seconds, and 3570 seconds.
  • The invalid-value regression cases fail against the unmodified Python source.
  • Black and Flake8 passed for the changed Python files; strict mypy passed for all 15 Python source files.

@mylee04
mylee04 requested a review from a team as a code owner September 25, 2026 03:53
pniedzielski
pniedzielski previously approved these changes Oct 16, 2026
Signed-off-by: Myles Lee <myungeun2dc@gmail.com>
Signed-off-by: Myles Lee <myungeun2dc@gmail.com>
@pniedzielski
pniedzielski force-pushed the fix/stats-dump-interval-validation branch from 69036c9 to b6e7632 Compare October 16, 2026 21:05

@pniedzielski pniedzielski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I've resolved a merge conflict brought about by merging #111; it was just documentation.)

@pniedzielski
pniedzielski merged commit 8bb96bd into bloomberg:main Oct 19, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Documented 60-minute stats_dump_interval aborts the Python process

2 participants