Make 'rbt status' work better with public local sites.

Review Request #5863 — Created May 22, 2014 and submitted

Information

RBTools
master
8f2ad81...

Reviewers

With public local sites, it's possible that the 'user' link in the session
resource will be broken. This is because the users resource only includes users
who are part of the given local site.

This change makes 'rbt status' work a little better in this case. We don't
actually need any data from the user resource, since all we're looking up here
is the username. In this case, we can just use the title attribute of the link,
which also happens to save us an API request.

  • Verified that 'rbt status' still works.
  • Ran unit tests.
Description From Last Updated

Can we call this get_link instead? Or get_link_raw or something?

chipx86chipx86

With this patch "resource.links" will return what we want, but "resource['links']" will fail. Generally we've allowed both for fields (e.g. …

SM smacleod
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/commands/status.py
        rbtools/api/resource.py
        rbtools/utils/users.py
      Ignored Files:
    
    
  2. 
      
reviewbot
  1. This is a review from Review Bot.
      Tool: Pyflakes
      Processed Files:
        rbtools/commands/status.py
        rbtools/api/resource.py
        rbtools/utils/users.py
      Ignored Files:
    
    
  2. 
      
chipx86
  1. 
      
  2. rbtools/api/resource.py (Diff revision 1)
     
     
    Show all issues

    Can we call this get_link instead? Or get_link_raw or something?

    1. I didn't want it to conflict with the autogenerated get_* methods

    2. Hmm, fair point.

      Crazy idea: How about making _links public?

    3. I considered that too. OK.

  3. 
      
david
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/commands/status.py
        rbtools/commands/patch.py
        rbtools/clients/__init__.py
        rbtools/api/resource.py
        rbtools/utils/users.py
      Ignored Files:
    
    
  2. 
      
reviewbot
  1. This is a review from Review Bot.
      Tool: Pyflakes
      Processed Files:
        rbtools/commands/status.py
        rbtools/commands/patch.py
        rbtools/clients/__init__.py
        rbtools/api/resource.py
        rbtools/utils/users.py
      Ignored Files:
    
    
  2. 
      
chipx86
  1. Looks good to me, but I want smacleod's feedback too.

  2. 
      
SM
  1. 
      
  2. rbtools/api/resource.py (Diff revision 2)
     
     

    Instead of making links public, how would you feel about capturing attribute access of links and giving out a "ResourceDictField" object. That way when the particular link is accessed you get a "ResourceLinkField" back. See https://github.com/reviewboard/rbtools/blob/b64f6a42cc2ec562b08904b9b861b173c3937d89/rbtools/api/resource.py#L206

  3. 
      
david
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/commands/status.py
        rbtools/api/resource.py
        rbtools/utils/users.py
      Ignored Files:
    
    
  2. 
      
reviewbot
  1. This is a review from Review Bot.
      Tool: Pyflakes
      Processed Files:
        rbtools/commands/status.py
        rbtools/api/resource.py
        rbtools/utils/users.py
      Ignored Files:
    
    
  2. 
      
SM
  1. I'm happy with the code, I think we should add some documentation though.

  2. rbtools/api/resource.py (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    With this patch "resource.links" will return what we want, but "resource['links']" will fail.

    Generally we've allowed both for fields (e.g. "review_request_resource['status']" and "review_request_resource.status") so there could be confusion if "links" doesn't work in the same way.

    I'm fine with treating links specially and only having ".links" work, but this is probably worth documenting.

    Could we also add documentation to the RBTools API docs about this new attribute?

  3. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-0.6.x (ddaa655)