Signed-off-by: Valerij Maljulin vmaljuli@redhat.com
Equality shouldn't be defined using the hash(). This would lead to some bad issues.
hash()
Maybe just return self.to_json() == other.to_json().
return self.to_json() == other.to_json()
How about return hash(tuple(sorted(self.to_json().items())))?
return hash(tuple(sorted(self.to_json().items())))
There're nested dicts sometimes which are unhashable. So I've used only fields I'm sure about it's hashability.
Can you put some more info in the commit message? Also the ref of the tk/card so it is easier to understand the change.
Why changing format here? You could just remove the duplicated elements and return a list again. You can turn it into a set and turn it back to a list. Doing like that you need to change all the tests...
Why changing format here? You could just remove the duplicated elements and return a list again.
This is good idea if we can do that after all the answers are fetched and omit having to work with sets and implementing custom hash methods.
def satisfied_and_unsatisfied_requirements(answers): satisfied = [] unsatisfied = [] for answer in answers: if answer.is_satisfied: requirements = satisfied else: requirements = unsatisfied requirement = answer.to_json() if requirement not in requirements: requirements.append(requirement) return satisfied, unsatisfied
rebased onto bab4bdda7b2c5166a85d04e9d1ce7171408eaea0
rebased onto c143357d19446b96bbaf326565bd45be2eca714e
This will break silently if I add new Answer subclass with additional field.
It will be better to remove duplicates when to_json() call is required in greenwave/api_v1.py to avoid all this additional code (as mentioned in my previous comment).
to_json()
greenwave/api_v1.py
Why would we need any new class here? No, it won't break. And of course it could be overrided by your class if you want.
The white list contains fields that are part of some dicts values returned from an overridden Answer.to_json() (Answer shouldn't know about these fields). Later, we may need to create a new subclass that has different fields, making the hash() function ineffective.
Answer.to_json()
Answer
rebased onto 8c5b9f20f381429a83691b91ba8486b66ffa021c
Looks good to me.
Remove.
+1 Just a minor comment (unused variable).
rebased onto 0a5fd98028aab4e4d881ab19d6f628df78079042
Pull-Request has been merged by vmaljulin
Signed-off-by: Valerij Maljulin vmaljuli@redhat.com