Skip to content

added preheating#530

Merged
LinoGiger merged 2 commits intomainfrom
feat/RAPID-7409-preheating-method
Apr 16, 2026
Merged

added preheating#530
LinoGiger merged 2 commits intomainfrom
feat/RAPID-7409-preheating-method

Conversation

@LinoGiger
Copy link
Copy Markdown
Collaborator

No description provided.

@claude
Copy link
Copy Markdown

claude bot commented Apr 16, 2026

Code Review for PR 530 - added preheating

Overview: This PR adds a preheat() method to RapidataFlowManager that calls /campaign/boost/preheat, warming up the boost system to reduce latency for upcoming flow items. It also bumps the version from 3.7.1 to 3.7.3.

Positives:

  • Clean, minimal implementation
  • Follows existing patterns: tracer span naming and logger.debug call match the rest of the class

Issues:

  1. Missing documentation (violates CLAUDE.md)
    CLAUDE.md states: when updating any interfaces make sure you update the docs and examples. docs/flows.md has no mention of preheat(), and there is no usage example. What the boost system is, when to call preheat(), and what effect it has are all non-obvious to SDK users.

  2. Docstring is too thin
    The docstring does not explain when to call this (e.g. before submitting a batch?), how much latency it reduces, or whether there is any cost or side-effect.

  3. Version skip: 3.7.1 to 3.7.3
    uv.lock jumps from 3.7.1 to 3.7.3, skipping 3.7.2. If 3.7.2 was already published to PyPI this is fine, but if not it may confuse consumers following the changelog.

  4. Cross-service access (minor)
    FlowManager directly accesses self._openapi_service.campaign.campaign_api, which is the only place in the flow module that touches the campaign API. A brief comment explaining why the campaign API is used here rather than a flow API would help future maintainers.

Potential Risks:
The endpoint can return 400, 401, and 403. These are handled by RapidataApiClient, so no direct issue. However since the method returns None, callers have no way to verify success short of catching an exception.

Test Coverage:
No tests included. A unit test mocking campaign_boost_preheat_post() and verifying it is called would be a good baseline.

Summary:
The implementation is correct and follows project conventions. Main gaps: (1) docs/flows.md needs a preheat section per CLAUDE.md, (2) the version skip from 3.7.1 to 3.7.3 should be intentional, (3) a comment on why campaign_api is used inside FlowManager would aid maintainability.

@claude
Copy link
Copy Markdown

claude bot commented Apr 16, 2026

Code Review: PR #530 — Added Preheating

Overview

This PR adds a preheat() method to RapidataFlowManager that calls the campaign_boost_preheat_post endpoint, along with documentation in docs/flows.md. Small, focused change.


Code Quality

rapidata_flow_manager.py — the new preheat method

The implementation follows the existing patterns in the file consistently:

  • Uses the tracer span correctly (RapidataFlowManager.preheat) matching the naming convention of other methods.
  • Uses logger.debug for the log message, consistent with the rest of the manager.
  • Returns None with correct type annotation.

One minor issue: the method lives on RapidataFlowManager but calls self._openapi_service.campaign.campaign_api — this is the only place in the entire rapidata_client/ tree that touches _openapi_service.campaign. All other flow-related API calls go through _openapi_service.flow.*. This cross-service coupling is a bit surprising and worth a brief comment explaining why preheat uses the campaign API rather than the flow API.


Documentation (docs/flows.md)

The new section is clear and well-placed (between config update and retrieval). One suggestion:

The phrase "warms up internal resources" is a bit vague for an SDK doc. Consider being more concrete, e.g. "pre-allocates annotator capacity" or whatever the actual backend effect is. This helps users judge when to call it.


Potential Issues

  • Cross-service API call: The method is on RapidataFlowManager but uses the campaign service — the only such cross-service call in the entire rapidata_client/ package. This works fine but is non-obvious; a one-line comment would help future maintainers.
  • No error handling: The underlying API can return 400 ValidationProblemDetails. The auto-generated client raises on non-2xx, which is consistent with other call sites in the codebase — just worth being aware of.
  • Version jump in uv.lock: Bumps from 3.7.1 to 3.7.3 (skipping 3.7.2). Confirmed pyproject.toml matches. Intentional skip?

Test Coverage

No tests are added for the new preheat() method. Given how simple it is (one API call, no branching logic), a single unit test asserting campaign_boost_preheat_post is called would be a good addition and would guard against regressions (e.g. accidentally swapping the API path).


Summary

Area Status
Code correctness OK
Follows project conventions OK — consistent with existing patterns
Cross-service coupling Worth a brief inline comment
Documentation Clear and well-placed; minor wording suggestion
Test coverage No tests added
Type safety Correct -> None annotation, tracer/logger usage is fine

Overall a clean, minimal addition. The main items worth addressing before merging are the missing test and a short comment explaining why campaign_api is used here instead of a flow API endpoint.

@LinoGiger LinoGiger merged commit cc83ee5 into main Apr 16, 2026
2 checks passed
@LinoGiger LinoGiger deleted the feat/RAPID-7409-preheating-method branch April 16, 2026 09:17
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