Merge ~sylvain-pineau/checkbox-ng:fix-1898900 into checkbox-ng:master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Sylvain Pineau | ||||
Approved revision: | e048ae62c126480369fb70c52629a40098916d22 | ||||
Merged at revision: | 0d597d612a0b0e67622e187684d2176b506172f9 | ||||
Proposed branch: | ~sylvain-pineau/checkbox-ng:fix-1898900 | ||||
Merge into: | checkbox-ng:master | ||||
Diff against target: |
71 lines (+6/-7) 4 files modified
plainbox/impl/ctrl.py (+1/-1) plainbox/impl/resource.py (+1/-1) plainbox/impl/test_ctrl.py (+1/-1) plainbox/impl/test_resource.py (+3/-4) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jonathan Cave (community) | Approve | ||
Sylvain Pineau (community) | Needs Resubmitting | ||
Review via email: mp+391979@code.launchpad.net |
Commit message
resource: Return an empty string when trying to access non-existent resource attributes
Instead of raising AttributeError which makes the full expression line to fail even is some sub expressions could pass.
Description of the change
Fixes the linked bug by making an invalid getattr expression non fatal, i.e not raising an AttributeError, which makes the full resource expression to evaluate to False even if some sub parts could return true.
Example:
package.foo == 'bar' or snap.name == 'core'
foo does not exist and even is the core snap is present, the requirement will fail.
Proposed solution is to return an empty string when trying to access non existent attr to properly fail the sub-expression.
Tested with:
https:/
This seems logical to me.
I'm trying to think of situations where it might fall down, the only situation I can think of is if a job was expecting a resource to be present but empty i.e.
something.a_field == ''
i.e. if 'a_field' was missing this would evaluate to True. However, I'm not aware that any resource would do this and the requirement really makes little practical sense