Skip to content
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

Open
dcalacci opened this issue Jan 22, 2015 · 5 comments
Open

coupling with funf #38

dcalacci opened this issue Jan 22, 2015 · 5 comments

Comments

@dcalacci
Copy link

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 a mapping field that directly relates to funf data:

        self.mapping = {'ActivityProbe': 'activity_probe',
                        'SmsProbe': 'sms_probe',
                        'CallLogProbe': 'call_log_probe',
                        'BluetoothProbe': 'bluetooth_probe',
                        'WifiProbe': 'wifi_probe',
                        'LocationProbe': 'simple_location_probe',
                        'ScreenProbe': 'screen_probe',
                        'RunningApplicationsProbe': 'running_applications_probe',
                        'HardwareInfoProbe': 'hardware_info_probe',
                        'AppUsageProbe': 'app_usage_probe'}

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!

@RogerTangos
Copy link
Contributor

Hey Dan - it really should be decoupled. The only reason things are so
non-orthogonal is that it was originally made with funf in mind.

Albert Carter

Programmer
Big Data Initiative
CSAIL, MIT

On Thu, Jan 22, 2015 at 4:37 PM, Dan Calacci [email protected]
wrote:

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
a mapping field that directly relates to funf data:

    self.mapping = {'ActivityProbe': 'activity_probe',
                    'SmsProbe': 'sms_probe',
                    'CallLogProbe': 'call_log_probe',
                    'BluetoothProbe': 'bluetooth_probe',
                    'WifiProbe': 'wifi_probe',
                    'LocationProbe': 'simple_location_probe',
                    'ScreenProbe': 'screen_probe',
                    'RunningApplicationsProbe': 'running_applications_probe',
                    'HardwareInfoProbe': 'hardware_info_probe',
                    'AppUsageProbe': 'app_usage_probe'}

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!


Reply to this email directly or view it on GitHub
#38.

@brian717
Copy link
Member

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.

@dcalacci
Copy link
Author

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.

@brian717
Copy link
Member

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.

@dcalacci
Copy link
Author

what you just described is nice and extensible, but this seems like it will work for now, I'll move forward

😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants