Fix up posting some changes including unicode with Python 2.x

Review Request #9748 — Created March 5, 2018 and submitted

Information

RBTools
master
7810223...

Reviewers

The changes I had made to make things encode correctly with Python 3
regressed posting diffs with unicode characters under Python 2. After
some investigation, what's needed is to use the native string type
(str) for headers and URLs passed into urllib2, rather than either
unicode or bytes.

  • Posted a change including an emoji under both Python 2.7 and 3.6.
  • Ran unit tests under Python 2.7 and 3.6.
Description From Last Updated

Can we do the same with method, and maybe even normalize headers? I can see this coming up a lot …

chipx86chipx86

testtesttest

TE testaaaaaa

Since we're Python 2.7 now, this can be: self.headers = { str(key): str(value) for key, value in six.iteritems(headers) }

chipx86chipx86

Same as above.

chipx86chipx86

Is this still needed now that HttpRequest normalizes?

chipx86chipx86
TE
  1. 
      
  2. rbtools/api/resource.py (Diff revision 1)
     
     
    Show all issues

    testtesttest

    1. This is a production server. Please use demo.reviewboard.org for testing, not this server.

  3. 
      
TE
  1. Ship It!

  2. 
      
chipx86
  1. 
      
  2. rbtools/api/request.py (Diff revision 1)
     
     
     
     
    Show all issues

    Can we do the same with method, and maybe even normalize headers? I can see this coming up a lot with contributed/third-party code, and it'd be nice to just cast things in one place and have it be done.

  3. 
      
david
chipx86
  1. 
      
  2. rbtools/api/request.py (Diff revision 2)
     
     
     
     
     
    Show all issues

    Since we're Python 2.7 now, this can be:

    self.headers = {
        str(key): str(value)
        for key, value in six.iteritems(headers)
    }
    
  3. rbtools/api/request.py (Diff revision 2)
     
     
     
     
     
    Show all issues

    Same as above.

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

    Is this still needed now that HttpRequest normalizes?

    1. HttpRequest only normalizes in its constructor. In this case we're replacing it after the fact.

    2. Maybe provide a @property setter so that it will be normalized both in the constructor and whenever overridden?

    3. Err... I don't think a property setter can shadow a value. I could name it something else but that would be just as fragile for callers because they'd need to know to set the special one.

    4. You'd just have _method under the hood, a method property that wraps it. It's transparent to callers.

    5. That might work fine for HttpRequest but not for Request (which inherits from urllib.request.Request).

    6. Request returns the method string through get_method(), rather than through an attribute. That can return a normalized version easily without it needing to be normalized up-front.

  5. 
      
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to master (636c83e)