Summary

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

Review Request #9748 — Created March 5, 2018 and submitted March 16, 2018, 4:41 p.m.

Information

Owner

david

Repository

RBTools

Branch

master

Bugs

Depends On

Commit

7810223...

Reviewers

Groups

rbtools

People

Description

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.

Testing Done


Posted a change including an emoji under both Python 2.7 and 3.6.
Ran unit tests under Python 2.7 and 3.6.

Issues

Description	From	Last Updated
Can we do the same with method, and maybe even normalize headers? I can see this coming up a lot …	chipx86	March 7, 2018, 11:07 a.m.
testtesttest	TE testaaaaaa	March 6, 2018, 10:46 a.m.
Since we're Python 2.7 now, this can be: self.headers = { str(key): str(value) for key, value in six.iteritems(headers) }	chipx86	March 15, 2018, 4:17 p.m.
Same as above.	chipx86	March 15, 2018, 4:17 p.m.
Is this still needed now that HttpRequest normalizes?	chipx86	March 15, 2018, 9:20 p.m.

flake8 passed.

JSHint passed.

rbtools/api/resource.py (Diff revision 1)
The issue has been dropped. Show all issues
```
testtesttest
```
1. chipx86 March 5, 2018, 11:46 p.m.
  This is a production server. Please use demo.reviewboard.org for testing, not this server.

```
Ship It!
```

rbtools/api/request.py (Diff revision 1)

The issue has been resolved. 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.

Commit:

f870f33a0118046b05f0420e9c211b359ad4ace9

fe5e0141a06d7938703a234a6a27a8083f0431e0

Diff:

Revision 2 (+28 -19)

Show changes

	rbtools/api/request.py
	rbtools/api/resource.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/api/request.py (Diff revision 2)

The issue has been resolved. 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)
}

rbtools/api/request.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Same as above.
```

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

The issue has been resolved. Show all issues

Is this still needed now that HttpRequest normalizes?

david March 15, 2018, 4:18 p.m.

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

chipx86 March 15, 2018, 4:19 p.m.

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

david March 15, 2018, 4:21 p.m.

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.

chipx86 March 15, 2018, 4:23 p.m.

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

david March 15, 2018, 4:26 p.m.

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

chipx86 March 15, 2018, 4:27 p.m.

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.

Commit:

fe5e0141a06d7938703a234a6a27a8083f0431e0

78102230f4fc9e0aa1e8c2fef786b65ea10cc511

Diff:

Revision 3 (+34 -17)

Show changes

	rbtools/api/request.py
	rbtools/api/resource.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (636c83e)