[openstack-dev] [Neutron] Seeking opinions on scope of code refactoring...

Carl Baldwin carl at ecbaldwin.net
Fri May 23 22:09:06 UTC 2014


Paul,

On Fri, May 23, 2014 at 8:24 AM, Paul Michali (pcm) <pcm at cisco.com> wrote:
> Hi,
>
> I’m working on a task for a BP to separate validation from persistence logic
> in L3 services code (VPN currently), so that providers can override/extend
> the validation logic (before persistence).
>
> So I’ve separated the code for one of the create APIs, placed the default
> validation into an ABC class (as a non-abstract method) that the service
> drivers inherit from, and modified the plugin to invoke the validation
> function in the service driver, before doing the persistence step.
>
> The flow goes like this…
>
>     def create_vpnservice(self, context, vpnservice):
>         driver = self._get_driver_for_vpnservice(vpnservice)
>         driver.validate_create_vpnservice(context, vpnservice)
>         super(VPNDriverPlugin, self).create_vpnservice(context, vpnservice)
>         driver.apply_create_vpnservice(context, vpnservice)
>
> If the service driver has a validation routine, it’ll be invoked, otherwise,
> the default method in the ABC for the service driver will be called and will
> handle the “baseline” validation. I also renamed the service driver method
> that is used for applying the changes to the device driver as apply_*
> instead of using the same name as is used for persistence (e.g.
> create_vpnservice -> apply_create_vpnservice).
>
> The questions I have is…
>
> 1) Should I create new validation methods A) for every create (and update?)
> API (regardless of whether they currently have any validation logic, B) for
> resources that have some validation logic already, or C) only for resources
> where there are providers with different validation needs?  I was thinking
> (B), but would like to hear peoples’ thoughts.

I think B.  C may leave a little too much inconsistency.  A feels like
extra boiler-plate.  Would there be any benefit to creating a higher
level abstraction for the create/update API calls?  I'm not suggesting
you do so but if you did then you could add a validation method to
that interface with a default pass.  Otherwise, I'd stick with B until
there is a need for more.

> 2) I’ve added validation_* and modified the other service driver call to
> apply_*. Should I instead, use the ML2 terminology of pre commit_* and post
> commit_*? I personally favor the former, as it is more descriptive of what
> is happening in the methods, but I understand the desire for consistency
> with other code.

I'm on the fence.  ML2 is not where I'm most familiar and I don't know
the history behind that design.  Without considering ML2 and
consistency, I think I like your terminology better.

> 3) Should I create validation methods for code, where defaults are being set
> for missing (optional) information? For example, VPN IKE Policy lifetime
> being set to units=seconds, value=3600, if not set. Currently, provider
> implementations have same defaults, but could potentially use different
> defaults. The alternative is to leave this in the persistence code and not
> allow it to be changed. This could be deferred, if 1C is chosen above.

I'm tempted to say punt on this until there is a need for it.

Carl

> Looking forward to your thoughts...
>
>
> Thanks!
>
> PCM (Paul Michali)
>
> MAIL …..…. pcm at cisco.com
> IRC ……..… pcm_ (irc.freenode.com)
> TW ………... @pmichali
> GPG Key … 4525ECC253E31A83
> Fingerprint .. 307A 96BB 1A4C D2C7 931D 8D2D 4525 ECC2 53E3 1A83
>
>
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>



More information about the OpenStack-dev mailing list