> A branch recently went in that replaced all returns of fault-wrapped webob
> exceptions with raising of non-wrapped webob exceptions. Can you convert any
> of the "return faults.Fault(...)" to "raise ..."?

Aha.. that explains something. :)  Sure.

> I think we may want to take the approach of ignoring unsupported query params
> rather than raising an exception (that's how Glance does it). We should be
> liberal in what we accept. A lot of the _get_items / _items code can get
> cleaned back up if we don't need to explicitly check filter params. Same goes
> for check_option_permissions.

Ok.  I was thinking about v1.1 and trying to 'guess' at what I should do here based on the possible response codes to the query.  I agree with what you say, though.  I'd rather allow and ignore search options we don't understand.  That will clean things up slightly.  I still need to check for permissions on certain search items that should be admin-api only, but I can cut the 2 lists of options down to 1.. (list of options that require admin)

> 
> Power states belong to the compute layer, and server statuses belong to the
> OSAPI. I think we should remove any OSAPI status-related code from the
> power_states module and place it in either api.openstack.common or
> api.openstack.servers. What do you think?

I had the same debate.  I was going to move it to api.openstack.common.. until I realized that compute.api needs to send the 'status' to search by to child zones.  This means that compute.api would end up making a call into api.openstack.common to convert a power state to the status to search for.. and that didn't feel quite right.  I think I can solve that differently.  I can make the OS API pass the 'status' to search for through to compute.api (along with 'state' which is the translation of status to power state).  compute.api will ignore 'status' and use 'state' but it'll pass 'status' on to novaclient to talk to zones.

That or I could leave it the way it is. :)  Changing it probably makes sense so there's clear separation.


>
> What is the 'fresh' query filter? I just started seeing that in novaclient
> requests.

Honestly, I have no clue. :-/  But I know that novaclient sends it.  I figured it was v1.0 equiv of 'changes-since' that's in v1.1.

> 
> Are there future plans to implement changes-since? I can see it is an accepted
> filter, just not respected.

Yeah, good question.  I'm already going beyond the scope of the story that was given to us for the sprint, and that one seemed a little more difficult.  Wasn't sure if compute.api should filter that.. or let OS API filter on the return to the client.  I thought maybe titan would have a better idea of how that should be done. ;)

> 
> In the DB API, why is there a _get_all_by_column_regexp and a
> _get_all_by_instance_name? Can't we use get_all_by_column_regexp to accomplish
> the name filter?

Let me double check that.  I saw a problem early on with this, because 'name' is a @property in the Instance class and I seemed to be getting back a property object with getattr.  I bet I was doing testing with a class and not an instance of a class at the time, however.  I bet this can be consolidated.

> 
> Can you change _get_all_by_column to _get_all_by_column_value? The former
> doesn't clearly say you are checking the value in a specific column when there
> is another function with column_regexp.

Sure.  Agree.

> 
> I don't see it necessary to list OSAPI-specific filters in the compute api.
> Can we fix that known_params?

Well, I need a table to map the search parameter to the function to call with its arguments.  In the initial merge prop, I didn't have this as I used getattr(self, '_get_all_by_%s' % opt_name).. and called that function.  The problem is that the generic 'column' ones need an extra argument.  The original merge prop had a list of those search options that could be done generically.  I could change it back... but it'd also require moving all of the _get_all_by* calls back outside of 'def get_all' so I could use getattr(self, ..) again.

Hm.. did that make sense?  We can chat on IRC to clarify if needed.

> 
> I really want to be able to sort by multiple parameters, and I think most
> people using the API would expect to be able to do that. How hard would it be
> to support this?

Yeah, I hear you.  The story I was given says to throw an error if multiple are given, so that's the main reason why I didn't code support for it. :)  My instinct is to say it's rather difficult... but when I really start to think about it, I'm not sure it's so bad.  I can think about it some more while doing the other fixups.

> 
> Don't get me wrong here, I'm really excited to get this feature in. Having
> done this in Glance already, I know this isn't a small undertaking :)

No worries. :)  It's difficult because I want it to perform... but I also want it to be flexible.  And I want it to be easily extendable to other search options, hence the tables and the generic by_column search functions.  But, this is complicated by the fact that not everything we want to search by is stored in the database and that it's not extremely clear how the API should behave in certain cases (unknown options, multiple options, options that aren't in the spec, etc). :)