-
Notifications
You must be signed in to change notification settings - Fork 82
cortex: add addDataModel method #106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cortex: add addDataModel method #106
Conversation
| pnfo['form'] = name | ||
| fullprop = '%s:%s' % (name,prop) | ||
| self.formTufoByProp('syn:prop',fullprop,**pnfo) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that we didn't reuse this method to implement _saveCoreModels() because that routine first loads types, and then forms. seems this would minimize dependency issues, so its a better approach. but we can't replicate that here, since we only have access to one datamodel at a time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we would also want an addDataModels([ (name,modl), ... ]) API for use when you do have more than one with interconnected type deps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
synapse/cores/common.py
Outdated
| def addDataModel(self, modname, modl): | ||
| ''' | ||
| Store all types/forms/props from the given data model in the cortex. | ||
| This should be done on initialization of an empty cortex only. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% true... this could be done after the fact to add a new data model to an old cortex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you
synapse/cores/common.py
Outdated
| Example: | ||
| modname = 'synapse.models.foo' | ||
| core.addDataModel(modname, s_dyndep.tryDynFunc(modname+'.getDataModel')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont think i'd introduce the dyndep / module complexity here. it'd probably be better to just have a small model dict declared in the example and pass it in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, no problem.
on a semi-related note, where would you like the documentation on a model's schema? should it go here, since this is where the model is actually installed?
| pnfo['form'] = name | ||
| fullprop = '%s:%s' % (name,prop) | ||
| self.formTufoByProp('syn:prop',fullprop,**pnfo) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we would also want an addDataModels([ (name,modl), ... ]) API for use when you do have more than one with interconnected type deps?
| Example: | ||
| for item in self.getTufosByProp('syn:prop'): | ||
| self._initPropTufo(item) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well git's diff algorithm is totally failing here. this function is not touched. focus on addDataModel and addDataModels.
|
all recommendations addresses, awaiting further suggestions or reject/merge |
first pass at adding a cortex-level routine for registering data models.