Skip to content

NO-JIRA | fix: refactor inspector to use pipelines#175

Open
AvielSegev wants to merge 1 commit intokubev2v:mainfrom
AvielSegev:adopt-pipelines-seperate-inspector-and-inspection
Open

NO-JIRA | fix: refactor inspector to use pipelines#175
AvielSegev wants to merge 1 commit intokubev2v:mainfrom
AvielSegev:adopt-pipelines-seperate-inspector-and-inspection

Conversation

@AvielSegev
Copy link
Collaborator

Signed-off-by: Aviel Segev asegev@redhat.com

@AvielSegev AvielSegev requested a review from a team as a code owner March 18, 2026 11:08
@AvielSegev AvielSegev requested review from amalimov and ronenav March 18, 2026 11:08
@AvielSegev AvielSegev force-pushed the adopt-pipelines-seperate-inspector-and-inspection branch 16 times, most recently from e5bbb23 to 7ca731b Compare March 18, 2026 15:29
@AvielSegev AvielSegev force-pushed the adopt-pipelines-seperate-inspector-and-inspection branch 3 times, most recently from e4da41c to 3dbfb44 Compare March 19, 2026 13:59
Signed-off-by: Aviel Segev <asegev@redhat.com>
@AvielSegev AvielSegev force-pushed the adopt-pipelines-seperate-inspector-and-inspection branch from 3dbfb44 to 348a8bc Compare March 19, 2026 14:05
InspectionWorkBuilder func(id string) []models.WorkUnit[models.InspectionStatus, models.InspectionResult]
)

type InspectionService struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you don't want to export this service, you should do it the other way around.
inspectionService + methods public.
Currently, we're exporting the service without any public methods.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would drop all the mutex from this service. If the InspectorService is the only caller, it should be responsible for protecting it.
You can have problems with redundant locking or even worst having some locking order problems.

return i
}

func (i *InspectionService) add(vmIDs []string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It could be clearer if this accepts a single vm

Add(vmID string) error
Cancel(vmID string) error
Stop() -> stops all the vms.

The contract is simpler like this imo.

InspectionWorkBuilder func(id string) []models.WorkUnit[models.InspectionStatus, models.InspectionResult]
)

type InspectionService struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is a good idea to separate concerns, with inspectionService responsible for vm inspection and inspector for the rest—each one with its scheduler and type.


type (
InspectionPipeline = WorkPipeline[models.InspectionStatus, models.InspectionResult]
InspectionWorkBuilder func(id string) []models.WorkUnit[models.InspectionStatus, models.InspectionResult]
Copy link
Collaborator

Choose a reason for hiding this comment

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

this could be private

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