Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: stable/rocky-m3
Choose a base branch
from

Conversation

fwiesel
Copy link
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

@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

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

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

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