-
Notifications
You must be signed in to change notification settings - Fork 33
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
coupling with funf #38
Comments
Hey Dan - it really should be decoupled. The only reason things are so Albert Carter Programmer On Thu, Jan 22, 2015 at 4:37 PM, Dan Calacci [email protected]
|
To add to Al's comment - the whole AccessControl package is a bit messy, and a rewrite would do it some good. At the moment, most deployments ignore it completely (control via scopes is typically good enough). It currently uses a model / table with column names corresponding to different probe names in Funf, and this is where the mapping dictionary comes into play. It's not quite coupled to Funf (the data could come from anywhere), but it does use the naming convention from Funf (not ideal, but not too bad), and is fixed in the number of different data types it can support (a worse problem than the column names). A better way to do this (and remove the Funf naming convention) would be to store these settings as a dictionary in either the PDS backend (configurable independent of the Django ORM) or the Django ORM (as a property bag), with data types / probes as keys. Either storage method would work, if you're interested in taking this on. The ORM property bag is likely the faster route. |
hey, not 100% sure what you were thinking of when you said 'property bag', but I took a stab at it. wanted to see if what I did is agreeable before I go and update other parts of the code. |
This is almost exactly what I meant, though by property bag I was thinking of a separate table for keys / datatype names, and a table linking a settings object to a key / attrib to a value. What you've done achieves the same thing, and if attrib names, app_id, and lab_id are unique and we don't plan on changing them after the fact, it works just as well anyway. So... thumbs-up from me. Definitely a positive change. |
what you just described is nice and extensible, but this seems like it will work for now, I'll move forward 😂 |
Hey, new to the project, been cleaning up the code and migrating the codebase to django 1.7 and other stuff like that on my own fork.
I'm wondering why some modules -
AccessControlledInternalDataStore
in particular - are coupled so tightly with Funf. For example, that class has amapping
field that directly relates to funf data:If it's just because the first projects to use this have funf-based, that's fine, but it seems to me that it's in the project's best interest to decouple it entirely and abstract the datastore objects from the kind of data being stored.
If it's part of some design decision I'm not privy to, could you explain it to me?
thanks!
The text was updated successfully, but these errors were encountered: