rewrite db.service_get_all() to really get all services

Bug #723171 reported by Christian Berendt
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
Wishlist
Josh Kleinpeter

Bug Description

At the moment the method service_get_all can only return all disabled or all enabled services, but not _all_ services. This should be fixed.

def service_get_all(context, session=None, disabled=False):
    if not session:
        session = get_session()

    result = session.query(models.Service).\
                   filter_by(deleted=can_read_deleted(context)).\
                   filter_by(disabled=disabled).\
                   all()
    return result

Related branches

Revision history for this message
Thierry Carrez (ttx) wrote :

This would change the semantics of service_get_all, for example as used by _describe_availability_zones in nova/api/ec2/cloud.py... Why do you think it needs fixing ? What is broken with current situation ?

Changed in nova:
status: New → Incomplete
Revision history for this message
Thierry Carrez (ttx) wrote :

@Christian: any chance you could provide the requested information ? We can't really make progress on this without your input.

Revision history for this message
Christian Berendt (berendt) wrote :

Sure ;) I forgot this report.

IMO the definition of the method "service_get_all" is wrong, because all other methods named "FOO_get_all" will return all the objects and not only a subset (calling the methods without arguments). For example you call "floating_ip_get_all" and you will get all floating ips. But if you call "service_get_all()" you will only receive all enable services.

For the receiving of subsets of objects there are special methods "service_get_by_FOO" and "service_get_all_FOO". All other "FOO_get_all" only have the context parameter.

So I think the method "service_get_all" isn't conform to the other "FOO_get_all" methods and I don't like that inconsistence. You know the "broken windows theory" (http://en.wikipedia.org/wiki/Broken_windows_theory?

Revision history for this message
Thierry Carrez (ttx) wrote :

yes :)

Changed in nova:
importance: Undecided → Wishlist
status: Incomplete → Confirmed
Changed in nova:
assignee: nobody → Josh Kleinpeter (jkleinpeter)
status: Confirmed → In Progress
Changed in nova:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in nova:
milestone: none → 2011.2
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.