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

pyPElib django backend should be able to handle multiple apps and not be hardcoded #2

Open
msune opened this issue Jun 7, 2013 · 3 comments

Comments

@msune
Copy link

msune commented Jun 7, 2013

imported from: fp7-ofelia/ocf#96

C.Fernandez said:

Hard-coded drivers

Also, the PersistenceEngine class has a hard-coded list of drivers (_drivers) instead of reading the corresponding directory: pypelib/persistence/backends/.

Hard-coded settings

Models inside pyPElib driver's (e.g. RuleModel and RuleTableModel) include hard-coded settings:

class PolicyRuleModel(models.Model):
    class Meta:
            """Machine exportable class"""
            app_label = 'vt_manager'
            db_table = 'pypelib_RuleModel'

that should be kept out of the code and be intuitive and easily configurable by a user, e.g. in /etc/pypelib/conf.d/.

Dynamic settings loading

After improving the previous point the settings will be outside the library, thus it would be interesting ( though maybe a bit performance costly, if heavily used ) to dynamically load these before performing any operation.

That could be done in a common code for every driver, e.g. the method _getDriver inside the PersistenceEngine class. Since this method is called prior to perform any operation it seems a good common point to load the settings file.

Note: the load of settings should be coded in a separate module.

@msune
Copy link
Author

msune commented Jun 7, 2013

I think the right approach is to keep the complexity within the library and the application that uses pypelib, and not adding extra complexity with conf files.

The easiest way to deal with Django, I guess it would be to define current Django Persistence Backend models as "meta-models" like:

#...
#Django is required to run this model
class PolicyRuleTableModel(models.Model):
        class Meta:
                """RuleTable model class"""
                app_label = None
                db_table = None

#...

Define app_label and db_table as None. Then the application (e.g. OCF's VM manager) needs to define its own model wrappers for PolicyRuleTableModel and RuleModel like:

from pypelib.persistence.backends.django.PolicyRuleTableModel import PolicyRuleTableModel
class PolicyRuleTableModelWrapper(PolicyRuleTableModel):
        class Meta:
                """RuleTable model class"""
                app_label = 'vt_manager'
                db_table = 'vt_manager_rule_table_model'

#And so for the rest of models

This implies that Django.py needs to take into account that, for this application, the models that it should use are the wrappers, for instance during load() and save().

Instead of the template types as of now:

@staticmethod
    def load(tableName, mappings, parser):
        Django.logger.info('Django.load')
        try:
            Table =  PolicyRuleTableModel.objects.get(name = tableName) # Note wrapper should be used instad

The derived classes should be used, so I think some form of initialization would be needed, a call which should be done before any interaction with the pipeline library (regarding persistence) made to the Django instance:

from pypelib.persistence.backends.django.Django import pypelib_django

pypelib_django.init(PolicyRuleTableModelWrapper, RuleModelWrapper)

#Then is safe to call load an save, provided that the implementation uses the types defined by init().

Just an idea, let me know your thoughts guys.

marc

@CarolinaFernandez
Copy link
Member

It seems fine for me. Although I like the idea of configuration files, it might be more difficult to implement than what is really needed...

The last part (initialization) is the one less defined by now, maybe we should check more into this and perform a bit of testing/prior development to see how to deal with this specific aspect.

@msune
Copy link
Author

msune commented Jun 7, 2013

I do not like the idea of the config files, as I said, for a simple reason. A config file should be something that the end-user of the application can change to modify the behaviour of an application, not really the intended user of pypelib, which is the developer of the end-user application.

For such frameworks or libraries in general an API, like the init() one, is generally the prefered way to proceed.

I propose to grab the helloworld application of django, create 2 parallel hello worlds, and try to integrate the pypelib in both applications and see if they run at the same time. It should be fairly easy.

marc

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

No branches or pull requests

2 participants