Saggi Mizrahi has posted comments on this change.
Change subject: securable: refactor the scurable implementation
......................................................................
Patch Set 2:
(2 comments)
Also, why not just remove the whole securable non-sense.
Let's just explicitly validate like normal people.
....................................................
File vdsm/storage/securable.py
Line 32: def secured(cls):
Line 33: for fun, val in cls.__dict__.iteritems():
Line 34: if (callable(val) and getattr(val, SECURE_FIELD, True)
Line 35: and not fun.startswith("__") # skipping builtins
Line 36: and not fun == SECURE_METHOD):
I wrote the previous one so I obviously think it's better.
But just to explain:
- It's easier to mentally process positive conditionals.
- The rule I'm trying to convey is "skip X if" and not "wrap X when"
- They are logically identical but the former is what you actually think when
you modify this method.
Line 37: setattr(cls, fun, secure_method(val))
Line 38: cls.__securable__ = True
Line 39: return cls
Line 40:
Line 43: setattr(f, SECURE_FIELD, False)
Line 44: return f
Line 45:
Line 46:
Line 47: def secure_method(method):
Were there any talks about it?
Though I prefer the underscore style and use it for everything I pull out of
VDSM. Consistency is important.
Line 48: def wrapper(self, *args, **kwargs):
Line 49: if not hasattr(self, "__securable__"):
Line 50: raise RuntimeError("Secured object is not a securable")
Line 51:
--
To view, visit http://gerrit.ovirt.org/22115
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5a8be7536748481746795e27701fbecc7c3318c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
