Skip to content

Quiet the "No more splits" log noise for draining-style stages - #124

Open
nusense wants to merge 2 commits into
developfrom
features/rhatcher_quiet_exceptions
Open

Quiet the "No more splits" log noise for draining-style stages#124
nusense wants to merge 2 commits into
developfrom
features/rhatcher_quiet_exceptions

Conversation

@nusense

@nusense nusense commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

get_dataset_for() re-raises a split type's StopIteration (e.g. nfiles.peek() when a dataset has been fully sliced up) as a bare AssertionError("No more splits in this campaign."). For stages configured with default_clear_cronjob=False (expected to keep polling for new files to show up later, e.g. draining-style datasets), this fires on every unsuccessful cron poll, forever, until new data arrives or someone intervenes.

poms_method's error_rewrite has no specific handling for AssertionError, so it falls through to the generic catch-all, which logs a full ERROR-level traceback via logging.exception on every occurrence — noisy, repeated log spam for what is a routine, expected "nothing new yet" condition, not a bug:

2026-08-27 10:33:02,306 <1726829.139949663827520> rewriting:
Traceback (most recent call last):
  File "/home/poms/poms/webservice/StagesPOMS.py", line 1284, in get_dataset_for
    res = splitter.next()
  ...
AssertionError: No more splits in this campaign.

Fix

  • Introduce StagesPOMS.NoMoreSplits (a plain Exception subclass) and raise that instead of AssertionError. Deliberately scoped to this one condition rather than a blanket except AssertionError, since AssertionError is used throughout the codebase for genuine bug/misconfiguration checks (e.g. assert self.init_shrek_if_needed(ctx), "...") that should keep their full traceback logging.
  • Give NoMoreSplits its own except clause in poms_method.error_rewrite, positioned before the generic catch-all: logs a short line via logit.log instead of a full traceback, while still returning the same HTTPError(400, ...) to the client as before.
  • The exception message itself now identifies which campaign/stage it's about — "No more splits in campaign '<name>' stage '<name>' (campaign_stage_id=<id>)." — instead of the old generic "No more splits in this campaign." with no way to tell which one. Same message shows up both in the quiet log line and in the HTTP response.

Test plan

  • Trigger a launch (via cron or manually) against a stage whose dataset has been fully split and which has default_clear_cronjob=False, and confirm the log shows a single line naming the campaign/stage instead of a full traceback.
  • Confirm a stage with default_clear_cronjob=True still has its cron schedule cleared as before (unaffected by this change).
  • Confirm other, unrelated AssertionErrors elsewhere in the app still get full traceback logging as before.

🤖 Generated with Claude Code

nusense and others added 2 commits August 27, 2026 10:48
get_dataset_for() re-raised StopIteration (from a split type run out of
data, e.g. nfiles.peek()) as a bare AssertionError. For stages configured
with default_clear_cronjob=False (expected to keep polling for new files
to show up later), this fires on every unsuccessful cron poll, and
poms_method's error_rewrite has no specific handling for it, so it falls
through to the generic catch-all and logs a full ERROR-level traceback
via logging.exception on each occurrence -- for what is a routine,
expected "nothing new yet" condition, not a bug.

Introduce StagesPOMS.NoMoreSplits (a plain Exception subclass) and raise
that instead of AssertionError, then give it its own except clause in
poms_method.error_rewrite that logs a short line via logit.log instead of
a full traceback, while still returning the same HTTPError(400, ...) to
the client as before. Deliberately scoped to this one condition rather
than a blanket `except AssertionError`, since AssertionError is used
throughout the codebase for genuine bug/misconfiguration checks that
should keep their full traceback logging.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The quiet NoMoreSplits log line previously just repeated the generic
"No more splits in this campaign." text, giving no way to tell which
campaign/stage it was about. Build the message from camp (the
CampaignStage passed into get_dataset_for) instead: campaign name,
stage name, and campaign_stage_id. Same message now shows up both in
the quiet log line and in the HTTPError response to the caller.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@nusense
nusense requested review from goodenou and marcmengel August 27, 2026 20:08
@nusense

nusense commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

It would be hard to test this on the integration instance because it requires running campaigns with splits that are then exhausted. The code itself seems straightforward though.

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.

1 participant