Opened 3 years ago

Last modified 11 months ago

#14332 needs_review defect

Use new string formatting interface

Reported by: cypherpunks Owned by: cypherpunks
Priority: Medium Milestone:
Component: Core Tor/Chutney Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The Python documentation mentions issues with printf-style string formatting and recommends the new str.format() interface [0]. The new interface is used in some parts but not everywhere. The attached patch fixes this by using the new interface where printf-style is currently used.

Additionally, it solves several bugs in lib/chutney/TorNet.py caused by the printf-style separator (%) being outside of the print(). The bugs result in Chutney crashing when tor or tor-gencert is not in PATH and not specified through the environment variables.

[0] https://docs.python.org/3/library/stdtypes.html#printf-style-string-formatting

Child Tickets

Attachments (2)

0001-Replace-printf-style-formatting-with-the-newer-str.f.patch (20.6 KB) - added by cypherpunks 3 years ago.
0001-Use-the-new-string-formatting-interface.patch (31.0 KB) - added by cypherpunks 11 months ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 3 years ago by cypherpunks

Status: newneeds_review

comment:2 Changed 23 months ago by cypherpunks

Severity: Normal
Status: needs_reviewneeds_revision

The patch needs a rebase and a fixup because it's incomplete right now

comment:3 Changed 23 months ago by cypherpunks

Owner: changed from nickm to cypherpunks
Status: needs_revisionassigned

I will work on this once #18185 is reviewed and merged to avoid merge conflicts.

comment:4 in reply to:  description Changed 22 months ago by cypherpunks

Replying to cypherpunks:

Additionally, it solves several bugs in lib/chutney/TorNet.py caused by the printf-style separator (%) being outside of the print(). The bugs result in Chutney crashing when tor or tor-gencert is not in PATH and not specified through the environment variables.

This sub-issue has been fixed in #18341 but the arguments for the str.format() interface still stands.

comment:5 Changed 18 months ago by cypherpunks

Status: assignedneeds_review

Because #18185 got merged, I've resumed work on this ticket and attached a new patch for review.

comment:6 Changed 18 months ago by teor

Status: needs_reviewneeds_revision

This patch modifies code that was also modified in #18826, and likely also conflicts with #9087, which needs revision.

I'd like to merge this one last, because it's going to conflict with many other patches.

Would you mind revising this patch after #9087 is merged?

comment:7 in reply to:  6 Changed 18 months ago by cypherpunks

Replying to teor:

This patch modifies code that was also modified in #18826, and likely also conflicts with #9087, which needs revision.

I'd like to merge this one last, because it's going to conflict with many other patches.

Would you mind revising this patch after #9087 is merged?

I do not mind. I'll take a look at #9087 and see if i can help move progress along there.

comment:8 Changed 18 months ago by teor

Ok, I think everything outstanding is now merged (#9087, mainly).
I would be happy to accept a patch for this on the current master 70a3c67.

Changed 11 months ago by cypherpunks

comment:9 Changed 11 months ago by cypherpunks

I've attached a new patch which is rebased on c72a6526b64d33b01a0efd3bd7eada25be7eb8fa. The patch addresses more old style string formatting that were added in the meantime.

comment:10 Changed 11 months ago by cypherpunks

Status: needs_revisionneeds_review
Note: See TracTickets for help on using tickets.