Skip to content

Conversation

modax
Copy link
Contributor

@modax modax commented Mar 25, 2016

Actually this is a rewrite of fuzzy_compare which primarily make it
work with arrays.

Current implementation was not correct with more than one element in the
array. What is more, I believe that when comparing kubernetes
descriptors we should compare arrays by their content ignoring element
order
. That's because kubernetes will reorder elements in arrays (I
can't think of a case where element order is significant) after initial
descriptor upload.


This is 1:1 based on commit you already cherry-picked in gareth-kubernetes from my fork:
garethr/garethr-kubernetes@accf0b5

Actually this is a rewrite of fuzzy_compare which primarily make it
work with arrays.

Current implementation was not correct with more than one element in the
array. What is more, I believe that when comparing kubernetes
descriptors we should compare arrays by their content *ignoring element
order*. That's because kubernetes will reorder elements in arrays (I
can't think of a case where element order is significant) after initial
descriptor upload.

Based on
garethr/garethr-kubernetes@accf0b5
@masterzen
Copy link
Contributor

@modax,

I see at least one instance where array order is important: container command arguments. But we can possibly live with this drawback.

But, more important, your version seems incorrect to me.
In our tests, it seems it always compares all elements of is to only the first element of should, leaving out the other elements of should.

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