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
@@ -122,7 +123,7 @@ class TritonService {
void preModuleConstruction(edm::ModuleDescription const&);
void postModuleConstruction(edm::ModuleDescription const&);
void preModuleDestruction(edm::ModuleDescription const&);
void preBeginJob(edm::ProcessContext const&);
void preBeginJob(edm::PathsAndConsumesOfModulesBase const&, edm::ProcessContext const&);
Copy link

Choose a reason for hiding this comment

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

@trevin-lee This change would prevent the code from compiling. Are these some un-wanted changes from a developing branch.

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