Skip to content

[scheduler] Consolidate host_info/passes steps in filter & weigher#365

Open
fwiesel wants to merge 1 commit intostable/rocky-m3from
scheduler_filter_host_info_requiring_instance_ids_reuse
Open

[scheduler] Consolidate host_info/passes steps in filter & weigher#365
fwiesel wants to merge 1 commit intostable/rocky-m3from
scheduler_filter_host_info_requiring_instance_ids_reuse

Conversation

@fwiesel
Copy link
Copy Markdown
Member

@fwiesel fwiesel commented Oct 5, 2022

Both host_info_requiring_instance_ids as well as host_passes/_weigh_object had duplicated code for extracting the instance-ids needed By consolidating them we reduce the code duplication.

Change-Id: Icfc1d3e554ff0834dec35d52772996284dc0a5da

Both host_info_requiring_instance_ids as well as host_passes/_weigh_object
had duplicated code for extracting the instance-ids needed
By consolidating them we reduce the code duplication.

Change-Id: Icfc1d3e554ff0834dec35d52772996284dc0a5da
@fwiesel fwiesel force-pushed the scheduler_filter_host_info_requiring_instance_ids_reuse branch from fe75123 to 6b6a76d Compare October 5, 2022 14:29
Copy link
Copy Markdown

@grandchild grandchild left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

Sorry for taking so long to get to this, laptop trouble + vacation.

return set(spec_obj.get_scheduler_hint('different_host'))
different_host = spec_obj.get_scheduler_hint('different_host')
if not different_host:
return different_host
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do you want to return a set always? If you do, then I'd do it explicitly, or else risk returning None or even an empty string:

Suggested change
return different_host
return set()

return set(spec_obj.get_scheduler_hint('same_host'))
same_host = spec_obj.get_scheduler_hint('same_host')
if not same_host:
return same_host
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same as above.

Suggested change
return same_host
return set()

@joker-at-work
Copy link
Copy Markdown

Should we re-open this on xena?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants