Speed up rbt post -u.

Review Request #5099 — Created Dec. 11, 2013 and submitted

Information

RBTools
master

Reviewers

Speed up rbt post -u.

rbt post -u, in an effort to match draft summaries, was fetching every
opened draft for a user individually and pulling info from them. This
was slow, particularly with lots of pending review requests.

Now it uses the field expansion support in the API to fetch drafts in
one go, reducing the number of calls significantly.

Ran with --debug and saw that it wasn't fetching drafts anymore.

Tested with and without pending drafts. It matched correctly.

Description From Last Updated

access to a Resource's 'fields' attribute is something we shouldn't really be doing. The fact that it's not "private" is …

SM smacleod

Will this work properly? Remember, as of now ".draft" isn't a real dictionary. I don't believe I've defined a __nonzero__ …

SM smacleod
SM
  1. 
      
  2. rbtools/commands/post.py (Diff revision 1)
     
     

    You can access the data from a resource using both attributes,
    or key lookup:

    review_request.draft
    

    vs:

    review_request['draft']
    

    So, if you prefer this conditional could be written like so:

    if 'draft' in review_request:
    

    Just a heads up. Go with whatever you prefer.

    1. Using 'in' seems way more pythonic.

    2. So it turns out I had a bug here anyway. Because we expand the draft, there's always going to be a draft parameter. Instead, we want to check if it's empty or not.

  3. rbtools/commands/post.py (Diff revision 1)
     
     

    access to a Resource's 'fields' attribute is something we shouldn't
    really be doing. The fact that it's not "private" is just a mistake
    I made when restructuring the whole transport stuff. We should really
    rename it "_fields" and not touch it.

    You will have the same effect if you change this to:

    fields = review_request
    
  4. 
      
chipx86
SM
  1. 
      
  2. rbtools/commands/post.py (Diff revision 2)
     
     

    Will this work properly? Remember, as of now ".draft" isn't a real dictionary.
    I don't believe I've defined a __nonzero__ method on it so you might want to
    verify this.

    1. The way it's returned in the API, it's actually a list of dictionaries (of which there will be at most 1). So I'm testing if it's empty, basically.

      It seems to work (I posted a change to one with a draft, and posted an update to this review request without a draft), but I can spend more time playing with it.

    2. Bah, yeah, that's me brain farting. Since it's a list (and the ResourceListField's act correctly) it should work as expected.

  3. 
      
SM
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Loading...