Opened 4 years ago

Last modified 4 years ago

#9343 enhancement reopened

Add a ensureBytes() helper function

Reported by: Craig Rodrigues Owned by: Craig Rodrigues
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch:
Author:

Description (last modified by Craig Rodrigues)

It's useful to have a utility function to convert from unicode to bytes.

https://github.com/twisted/twisted/pull/961

There are several places in the Twisted source where a pattern like:

` if isinstance(s, unicode):

s = s.encode("ascii")

`

This attempts to encapsulate this pattern in a function.

Based on this feedback from an earlier PR, I did not put this in twisted.python.compat: https://github.com/twisted/twisted/pull/936#discussion_r153816654

My motivation for this is to fix some problems I found in twisted.web when I found this problem when trying to port https://github.com/LeastAuthority/txkube to Python 3: https://github.com/twisted/twisted/pull/936#discussion_r153823007

Specifically if I have: ` networkString(foo) ` if foo is of type bytes on Python 2, it will work fine, but if foo is of type bytes on Python 3, this networkString will throw an exception.

If this code goes in, I would also like to delete the _ensureBytes() private function in https://github.com/twisted/twisted/blob/trunk/src/twisted/web/http.py#L1154

Change History (17)

comment:1 Changed 4 years ago by Craig Rodrigues

Keywords: review added

comment:2 Changed 4 years ago by Craig Rodrigues

Description: modified (diff)

comment:3 Changed 4 years ago by Jason Litzinger

Owner: set to Jason Litzinger

Taking for review.

comment:4 Changed 4 years ago by Jason Litzinger

Keywords: review removed
Owner: changed from Jason Litzinger to Craig Rodrigues

comment:5 Changed 4 years ago by Jean-Paul Calderone

Please write more words on tickets. Like ... why do this?

comment:6 Changed 4 years ago by Craig Rodrigues

Description: modified (diff)

comment:7 Changed 4 years ago by Craig Rodrigues

Keywords: review added

comment:8 Changed 4 years ago by Craig Rodrigues

Description: modified (diff)
Summary: Add a toBytes() helper functionAdd a ensureBytes() helper function

comment:9 Changed 4 years ago by Craig Rodrigues

Submitting a new PR for this, renaming function to ensureBytes()

https://github.com/twisted/twisted/pull/961

Last edited 4 years ago by Craig Rodrigues (previous) (diff)

comment:10 Changed 4 years ago by Craig Rodrigues

Description: modified (diff)

comment:13 Changed 4 years ago by Craig Rodrigues <rodrigc@…>

Resolution: fixed
Status: newclosed

In 30b28fbc:

Error: Processor CommitTicketReference failed
 does not appear to be a Git repository. See the log for more information.

comment:14 Changed 4 years ago by mark williams

Keywords: review added

comment:15 Changed 4 years ago by Amber Brown <hawkowl@…>

In e94e592:

Error: Processor CommitTicketReference failed
 does not appear to be a Git repository. See the log for more information.

comment:16 Changed 4 years ago by hawkowl

Resolution: fixed
Status: closedreopened

Hmm. the ticket didn't do this automatically. Reopening, as this is a revert.

We should decide if we want this functionality or whether it allows potential bugs. I'm a -0.5, because it may be useful on some limited external APIs for compat reasons, but I also believe it makes the conversion too implicit for future debugging and maintainability, even if it is shorter.

comment:17 Changed 4 years ago by hawkowl

Keywords: review removed
Note: See TracTickets for help on using tickets.