On Mon, Jun 22, 2015 at 2:43 PM, Doug Hellmann <d...@doughellmann.com> wrote:
> Excerpts from Sean Dague's message of 2015-06-22 13:30:44 -0400: > > On 06/22/2015 01:15 PM, Michael Krotscheck wrote: > > > Having _just_ done this, a couple of suggestions. > > > > > > - If the middleware is NOT optional - that is, it provides some kind of > > > a fundamental component or specification of the API, like ETag caching, > > > CORS, or DB Session management - then the middleware should be added > > > during the app initialization routine, likely in the app factory. In > > > this case, we have tight control over the initialization order, and can > > > make sure that oslo.config is there first. Yay config. > > > > > > - If the middleware _is_ optional, then I really feel this problem is > > > solved by pastedeploy's filter_factory pattern. It lets you define as > > > many required parameters as you want, and spits back a middleware > > > instance. That way you have the freedom to set whatever configuration > > > options you want, or omit the middleware altogether. > > > > > > http://pythonpaste.org/deploy/#paste-filter-factory > > > > > > Ultimately, what you should want is to ask a factory method for a thing > > > with some configuration options, and it spits an instance back at you. > > > If your project doesn't support pastedeploy, that limits your options > > > (ahem-ironic-ahem). Otherwise, it's really a team decision on whether > > > something is a 'Fundamental feature' or something that's optional. > > > > > > In the case of Ceilometer? Sorry, but I think it's optional. You should > > > be able to run any component of openstack without ceilometer. So I > don't > > > really think it should depend on oslo_config. > > > > Ok, here is where I'm confused. Ceilometer middleware *already* depends > > on oslo_config. > > > > > https://github.com/openstack/ceilometermiddleware/blob/master/ceilometermiddleware/swift.py#L42 > > > > And it does stuff with it here: > > > > > https://github.com/openstack/ceilometermiddleware/blob/master/ceilometermiddleware/swift.py#L107 > > > > But regardless, it uses oslo_messaging, which uses oslo_config. So it > > seems like the only issue at hand is middleware retriggering conf > parsing. > > > > Because the current model of requiring simultaneous code lands in > > oslo_messaging and ceilometermiddleware, and simultaneous config updates > > in every installer and config management system to configure the same > > thing in 2 different ways, does seem like a long term win. And honestly > > just prohibits most of what people seem to want to do with new messaging > > approaches. > > I explained this on IRC, but I'll repeat it here as a more permanent > record. > > We should not have the ceilometer middleware (re)initializing > oslo.config. That combines 2 concerns (setting up configuration and > doing whatever ceilometermiddleware is already doing) in one piece > of middleware, and that will make it more difficult to combine > ceilometermiddleware with other middleware that also wants access > to the configuration settings. > > If the application code isn't setting up oslo.config, then we should > have another piece of middleware do just that one thing. It can take > options from paste's config to tell it how to do that work, and then > other middleware later in the pipeline can rely on oslo.config being set > up and the config files being loaded. > > Doug > > Whatever is decided for this, I assume it'll also apply to the auth_token middleware, since that uses oslo.config, too. We already have a bug[1] and a proposed change[2] that isn't passing jenkins. [1] https://bugs.launchpad.net/keystonemiddleware/+bug/1406218 [2] https://review.openstack.org/#/c/143063/ Brant
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev