Skip to content
This repository has been archived by the owner on Jan 24, 2018. It is now read-only.

[RFC] Improve Modularization #186

Open
lasley opened this issue Dec 14, 2016 · 1 comment
Open

[RFC] Improve Modularization #186

lasley opened this issue Dec 14, 2016 · 1 comment

Comments

@lasley
Copy link
Contributor

lasley commented Dec 14, 2016

This is going to be a big one, but it's kind of bugging me in the template creations so I'll probably handle it soon.

We need an interface, instead of overloading all the parent methods. The overloading isn't really scaling IMO when considering what the interpreter is going through in terms of logic.

Consider a standard deploy_base:

...
    def deploy_base(self):
        """ It backs up the file store """
        res = super(ClouderBackup, self).deploy_base()
        if self.base_id.application_id.type_id.name == 'elasticsearch':
            do_stuff()
        return res
...

Which is fine in itself, but the whole picture becomes an N+1 issue due to the recursive variable assignment, super method call, logic statement, return statement.

I propose instead that we use the core methods as an interface, then escalate to the adapter method based on its type:

...
    def deploy_base(self, *args, **kargs):
        return getattr(self, '%s_deploy_base' % MODULE_TYPE)(*args, **kwargs)
...
    def elasticsearch_deploy_base(self, *args, **kargs):
        return do_stuff()

To accomplish this, we need some sort of type declaration or registry that would work globally (e.g. elasticsearch). I'm not sure where we would get this though.

From there, we would also define a rule that I ambitiously put in my WIP ES template:

All public methods and properties are to be prefixed with elastic_ in
order to prevent namespace clashes with existing operations, unless
overloading and calling + returning the super.

Thoughts?

@YannickB
Copy link
Owner

I'm not really good at this so I don't know if there is a better method but your proposition looks good to me. I experimented trouble reading tracelog sometimes so I'm all for this improvement.

That said, please note that sometimes the conditions is a little more complex (https://github.com/clouder-community/clouder/blob/master/clouder_template_mail/template.py#L115 for example) but this shall not be a problem.

I would like to avoid this RFC to be addressed before we have a oneclick up and running, this is still a big refactoring, debuging oneclick shall be a priority right now.

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

No branches or pull requests

2 participants