Code review comment for lp:~cbehrens/nova/servers-search

Hi Chris!

Few little comments...

190 + # See if a valid search option was passed in.
191 + # Ignore unknown search options for possible forward compatability.
192 + # Raise an exception if more than 1 search option is specified
193 + found_opt = None
194 + for opt in exclusive_opts:
195 + v = search_opts.get(opt, None)
196 + if v:
197 + if found_opt is None:
198 + found_opt = opt
199 + else:
200 + LOG.error(_("More than 1 mutually exclusive "
201 + "search option specified (%(found_opt)s and "
202 + "%(opt)s were both specified") % locals())
203 + raise exception.InvalidInput(reason=_(
204 + "More than 1 mutually exclusive "
205 + "search option specified (%(found_opt)s and "
206 + "%(opt)s were both specified") % locals())

Might be more succinctly written as:

found_opt = None
found_opts = [opt for opt in exclusive_opts
              if search_opts.get(opt, None)]
if len(found_opts) > 1:
    found_opt_str = ", ".join(found_opts)
    msg = _("More than 1 mutually exclusive "
            "search option specified: %(found_opt_str)s")
            % locals())
    logger.error(msg)
    raise exception.InvalidInput(reason=msg)

if found_opts:
    found_opt = found_opts[0]

I recognize (and agree with your decision) not to do regexp matching via the database. Not only is it not portable, it's not any more efficient to do that at the database level (still requires a scan of all pre-restricted rows anyway...).

However this method signature:

+def instance_get_all_by_name_regexp(context, ipv6_regexp):

Looked strange, perhaps a copy/paste error? Should ipv6_regexp be name_regexp?

Cheers!
jay