• 
      

    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)