-
Notifications
You must be signed in to change notification settings - Fork 82
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
refact(RHINENG-15309): Refactor filtering logic related to custom staleness #2183
base: master
Are you sure you want to change the base?
Conversation
@@ -12,6 +12,7 @@ | |||
PRODUCER_ACKS = {"0": 0, "1": 1, "all": "all"} | |||
|
|||
HOST_TYPES = ["edge", None] | |||
ALL_STALENESS_STATES = ("fresh", "stale", "stale_warning") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a no possibility of checking "culled" state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All I'm doing is moving this constant from host_repository.py
to config.py
, not changing any values. But no, we don't allow querying hosts that are in the culled state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is needed so I filed RHINENG-15328.
@@ -170,6 +180,21 @@ def _staleness_filter( | |||
return [or_(*staleness_conditions)] | |||
|
|||
|
|||
def staleness_to_conditions( | |||
staleness, staleness_states, host_type, timestamp_filter_func, omit_host_type_filter: bool = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should stick with one name staleness_states
or staleness_type
? The caller calls it type but the argument uses states; unnecessary change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is retaining the same signature as what it had when it was in culling.py
.
Also, I don't see staleness_type
used anywhere; can you please point me to that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh it was staleness_types
on line 206, 207, and 228 in lib/host_repository.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Overview
This PR is being created to address RHINENG-15309.
The purpose of this PR is to simplify some things, but it also resolves an inconsistency with staleness_to_conditions, which is needed to unblock #2181.
PR Checklist
Secure Coding Practices Documentation Reference
You can find documentation on this checklist here.
Secure Coding Checklist