Skip to content

Conversation

@Sukuna0007Abhi
Copy link
Contributor

@Sukuna0007Abhi Sukuna0007Abhi commented Jan 16, 2026

This PR refactors collection request builders to use classes that enforce a consistent build(session, logger, enabled_phase_names, days_until_collect_again) signature, fixing issue #3272.

changed

  • Added CollectionRequestBuilder (abstract base class) that defines build(...).
  • Implemented concrete builders: PrimaryCollectionRequestBuilder, SecondaryCollectionRequestBuilder, FacadeCollectionRequestBuilder, MLCollectionRequestBuilder.
  • Added backward-compatible wrapper functions:
  • build_primary_repo_collect_request(...)
  • build_secondary_repo_collect_request(...)
  • build_facade_repo_collect_request(...)
  • build_ml_repo_collect_request(...)
  • Added tests: test_collection_builders.py to assert subclassing, build presence, correct signatures and wrapper parity.

Why

  • Ensures all collection request builders follow a consistent and enforceable API that accepts enabled_phase_names (as discussed in Use Classes to enforce consistent APIs #3272).
  • Keeps existing call sites working via thin wrappers — no runtime behavior changed.

Ready for review ...

Fixes #3272

@Sukuna0007Abhi Sukuna0007Abhi changed the title refactor(collections): use builder classes for collection request builders (issue #3272) refactor use builder classes for collection request builders (issue #3272) Jan 16, 2026
@nexpectArpit nexpectArpit force-pushed the fix/3272-use-classes-for-consistent-apis branch 2 times, most recently from cda052a to 3b21ab6 Compare January 16, 2026 09:37
…lders (issue chaoss#3272)

Signed-off-by: Sukuna0007Abhi <appsonly310@gmail.com>
@nexpectArpit nexpectArpit force-pushed the fix/3272-use-classes-for-consistent-apis branch from 3b21ab6 to 553b400 Compare January 16, 2026 09:49
@sgoggins sgoggins added the deployed version Live problems with deployed versions label Jan 20, 2026
@MoralCode MoralCode removed the deployed version Live problems with deployed versions label Jan 21, 2026
Copy link
Contributor

@MoralCode MoralCode left a comment

Choose a reason for hiding this comment

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

hmmm, maybe i wasnt expecially detailed in my underlying issue, or the fact that I said "classes" (plural) may have confused things.

@ABrain7710 is currently working on some improvements to task scheduling that will probably change these functions in a major way anyway, so I think that may give a better opportunity for refactoring this

@MoralCode MoralCode marked this pull request as draft January 21, 2026 16:44
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.

Use Classes to enforce consistent APIs

3 participants