Skip to content

Conversation

trevin-lee
Copy link

PR description:

  • Implement RetryActionDiffServer for SonicTriton using TritonService’s server registry; remove per-action alternate server parameters.
  • Use TritonClient::updateServer(TritonService::Server::fallbackName) to switch servers on retry, per review guidance.
  • Extend test coverage:
    • Add Catch2 unit test HeterogeneousCore/SonicTriton/test/test_RetryActionDiffServer.cc (arms → updateServer(fallback) → no-op on second retry → exception path is caught).
    • Extend HeterogeneousCore/SonicTriton/test/tritonTest_cfg.py with --retryAction {same,diff} and a verbose confirmation line.
    • Add scripted test TestHeterogeneousCoreSonicTritonRetryActionDiff_Log to assert the selected retry policy.
  • Move all test definitions to HeterogeneousCore/SonicTriton/test/BuildFile.xml (remove package-level test entries; use proper <bin> with catch2).
  • Provide a protected default constructor for TritonClient (test double support), removing the unused testing flag constructor.
  • Aligns with the Retry framework expectations discussed in review (ref: “Test PR for new Retry Framework Test PR for new Retry Framework #19”); no physics changes expected, functionality is exercised only when retry is configured.

PR validation:

  • Built and ran unit/integration tests in a fresh CMSSW_15_1_0_pre3 area:
    • scram b -j 8
    • scram b runtests TEST=HeterogeneousCore/SonicTriton (passes)
  • Catch2 test validates action behavior; cmsRun-based tests validate configuration wiring for both same and diff policies.
  • No changes to standard workflows unless Client.Retry is explicitly configured.

Backport:

  • Not a backport. If needed, a follow-up backport can be proposed for CMSSW_15_0_X after this PR is reviewed.

…end tests; add Catch2 unit test; update tritonTest_cfg.py for retry policy logging; ported to CMSSW_15_1_0_pre3 base
@kpedro88
Copy link

@trevin-lee I've updated the master branch to synchronize with upstream cmssw now, but you may have to remake the PR (GitHub sometimes doesn't update correctly)

@kpedro88 kpedro88 changed the base branch from master to CMSSW_15_0_X August 29, 2025 14:50
@kpedro88 kpedro88 changed the base branch from CMSSW_15_0_X to master August 29, 2025 14:50
@trevin-lee trevin-lee closed this Sep 4, 2025
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.

2 participants