Code review comment for lp:~vishvananda/nova/has_role_cache

Rick Harris (rconradharris) wrote :

> 62 + if rslt == None:

Minor nit, but better as, `if rslt is None`.

> 50 + role_key = str("rolecache-%s-%s" % (User.safe_id(user), role))
> 51 + if project:
> 52 + return "%s-%s" % (role_key, Project.safe_id(project))
> 53 + return role_key

Nothing wrong with the code above, but, it looks like this case is calling out for a `join`:

key_parts = ['rolecache', User.safe_id(user), str(role)]
if project:
  key_parts.append(Project.safe_id(project))
return '-'.join(key_parts)

review: Needs Fixing

« Back to merge proposal