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
ZEN-26894 fix production state sort error #2177
ZEN-26894 fix production state sort error #2177
Conversation
337294d
to
444ee78
Compare
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.
We need to evaluate the performance impact. This change will make sorting on production state perform the same as sorting on an un-indexed field.
Filtering on productionState may be worse than filtering on an un-indexed field.
The biggest impact is when start and limit are defined, as when a user is scrolling/paging through results. Since we don't store productionState in the catalog, this will require that we unbrain the entire device list everytime we load 1 "page" of results.
Products/Zuul/facades/__init__.py
Outdated
# Unbraining these brains makes them no longer brains - so set the SearchResults flag accordingly | ||
areBrains = False | ||
|
||
psFilteredBrains = [u for u in catbrains if psManager.getProductionStateFromGUID(u.getUUID()) in prodStates] |
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.
since we have unbrained them, we can just do u.getProductionState() and eliminate the need for psManager.
Products/Zuul/facades/__init__.py
Outdated
else: | ||
sortedBrains = psFilteredbrains | ||
for u in psFilteredBrains: | ||
prodState = psManager.getProductionStateFromGUID(u.getUUID()) |
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.
Same as above, use u.getProductionState()
self.assertEquals(device.getProductionState(), 1000) | ||
self.assertEquals(device.location.getRelatedId(), "test1") | ||
|
||
def test_deviceSortByProdStateWithLocationFilterReturnsCorrectDevicesAndSortsCorrectly(self): |
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.
We need a test where the sort is by an unindexed field and the filter is productionState, as described in ZEN-26901
05441fa
to
800a2c3
Compare
I have updated this PR with the latest discussed changes - which are much simpler. |
edd5602
to
5d2e55f
Compare
35a37bb
to
e2d0f67
Compare
retest this please |
jenkins please test this |
In the unit tests for this set of fixes:
collector
is used as a non-indexed fieldname
is used as an index field