Epiphyte synprop req #409
Epiphyte synprop req #409
Conversation
synapse/models/crypto.py
Outdated
| @@ -8,6 +14,19 @@ | |||
|
|
|||
| class CryptoMod(CoreModule): | |||
|
|
|||
| @modelrev('crypto', 201708231712) | |||
| def _revModl201708231712(self): | |||
| node = self.core.formTufoByProp('syn:prop', 'rsa:key:mod') | |||
There was a problem hiding this comment.
wont this always create it?
synapse/models/inet.py
Outdated
| @@ -411,6 +411,18 @@ def _revModl201708141516(self): | |||
| if 'syn:type:fields' not in node[1]: | |||
| self.core.addRows([(node[0], 'syn:type:fields', 'url=inet:url,file=file:bytes', s_common.now())]) | |||
|
|
|||
| @modelrev('inet', 201708231646) | |||
| def _revModl201708231646(self): | |||
| node = self.core.formTufoByProp('syn:prop', 'inet:ipv4:type') | |||
|
This has been tested against the content of #400 and it does work there. |
Codecov Report
@@ Coverage Diff @@
## master #409 +/- ##
==========================================
+ Coverage 89.93% 90.49% +0.56%
==========================================
Files 127 127
Lines 14312 14336 +24
==========================================
+ Hits 12871 12974 +103
+ Misses 1441 1362 -79
Continue to review full report at Codecov.
|
| @@ -1878,6 +1881,21 @@ def _runAutoAdd(self, toadd): | |||
| continue | |||
| self.formTufoByProp(form, valu) | |||
|
|
|||
| def _reqProps(self, form, fulls): | |||
| if not self.enforce: | |||
There was a problem hiding this comment.
So if enforce mode is off, required props are not required. (thinking aloud)
There was a problem hiding this comment.
@vertexmc this is per @invisig0th
synapse/cores/common.py
Outdated
| # despite the model requiring a ptype to be present. | ||
| props.pop('syn:prop:ptype', None) | ||
|
|
||
| for prop in props: |
There was a problem hiding this comment.
to avoid the loops in python ( and make them be in C ) you could probably do something like
propset = set(props)
fullset = set(fulls)
missing = propset - fullset
if missing:
etc etc etc
There was a problem hiding this comment.
This is fixed; along with a early return if there are no req'd props on the form.
| @@ -100,6 +100,7 @@ def __init__(self, load=True): | |||
| self.props = {} | |||
| self.forms = set() | |||
|
|
|||
| self.reqprops = collections.defaultdict(list) | |||
There was a problem hiding this comment.
good call on keeping these in an optimized lookup
| @@ -121,7 +121,7 @@ class NoInitCore(Exception): pass # API disabled because no cortex | |||
| class NoCurrSess(Exception): pass # API requires a current session | |||
|
|
|||
| class SidNotFound(Exception): pass | |||
| class PropNotFound(Exception): pass | |||
| class PropNotFound(SynErr): pass | |||
There was a problem hiding this comment.
We should create a story to make all of these Exceptions SynErrs
There was a problem hiding this comment.
@vertexmc we do have one in our backlog
synapse/tests/test_model_inet.py
Outdated
| self.eq(pdef[1].get('syn:prop:ptype'), 'str') | ||
|
|
||
| # Now make a node | ||
| tufo = core.formTufoByProp('inet:ipv4', '192.168.1.1', type='uni') |
There was a problem hiding this comment.
technically type for this ipv4 would be priv ;)
synapse/models/crypto.py
Outdated
| def _revModl201708231712(self): | ||
| node = self.core.getTufoByProp('syn:prop', 'rsa:key:mod') | ||
| if not node: # pragma: no cover | ||
| # Its possible someone deleted their syn:prop=inet:ipv4:type node :( |
| @@ -125,10 +126,11 @@ def __init__(self, load=True): | |||
| self.addTufoProp('syn:form', 'ptype', ptype='syn:type', req=1, doc='Synapse type for this form') | |||
|
|
|||
| self.addTufoForm('syn:prop', ptype='syn:prop') | |||
| self.addTufoProp('syn:prop', 'doc', ptype='str', req=1, doc='Description of the property definition') | |||
| # TODO - Re-enable syn:prop:doc req = 1 after cleaning up property docstrings. | |||
There was a problem hiding this comment.
I'd add another story to track adding docstrings for all props and enabling this. We should inform users before enabling this to avoid private models breaking.
There was a problem hiding this comment.
@therealsilence has a story for doing all these docs already.
There was a problem hiding this comment.
Looks great. I had one change and agree with @invisig0th 's request changes though.
… as set comparison instead.
…saves round tripping the storage layer for prop setting and allows us to make nodes with required props. This removes the gest:prog act=set event.
Enforce req=1 on node properties during node creation.