Opened 6 years ago
Closed 5 years ago
#5809 enhancement closed wontfix (wontfix)
replace usage of file with FilePath in twisted.mail
Reported by: | Thijs Triemstra | Owned by: | |
---|---|---|---|
Priority: | low | Milestone: | Python-3.x |
Component: | Keywords: | easy | |
Cc: | Thijs Triemstra, moijes12 | Branch: |
branches/mail-filepath-5809
branch-diff, diff-cov, branch-cov, buildbot |
Author: | thijs |
Description (last modified by )
The usage of file
should be replaced with FilePath
in twisted.mail and it's tests for Python 3 compatibility.
twisted/mail/test/test_options.py:31: aliasFile = file(self.aliasFilename, 'w') twisted/mail/test/test_mail.py:169: self.f = file(self.name, 'w') twisted/mail/test/test_mail.py:540: fObj = file(j(self.d, f), 'w') twisted/mail/test/test_mail.py:856: f = file(name + '-H', 'w') twisted/mail/test/test_mail.py:860: f = file(name + '-D', 'w') twisted/mail/test/test_mail.py:1693: lines = file(tmpfile).readlines() twisted/mail/test/test_mail.py:1845: lines = file('process.alias.out').readlines() twisted/mail/test/test_imap.py:1563: message = file(infile) twisted/mail/alias.py:70: fp = file(filename) twisted/mail/alias.py:155: f = file(self.finalname, 'a') twisted/mail/alias.py:403: f = file(addr[1:])
Attachments (3)
Change History (18)
comment:1 Changed 6 years ago by
Description: | modified (diff) |
---|
Changed 6 years ago by
Attachment: | mail-open-5809.patch added |
---|
comment:2 Changed 6 years ago by
Keywords: | review added |
---|
comment:3 Changed 6 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Thijs Triemstra |
comment:4 Changed 6 years ago by
Description: | modified (diff) |
---|---|
Status: | new → assigned |
Summary: | replace usage of file with open in twisted.mail → replace usage of file with FilePath in twisted.mail |
comment:5 Changed 6 years ago by
Author: | → thijs |
---|---|
Branch: | → branches/mail-filepath-5809 |
(In [35027]) Branching to 'mail-filepath-5809'
comment:6 Changed 6 years ago by
comment:7 Changed 6 years ago by
Keywords: | review added |
---|---|
Owner: | Thijs Triemstra deleted |
Status: | assigned → new |
comment:9 Changed 6 years ago by
Owner: | set to argonemyth |
---|---|
Status: | new → assigned |
comment:10 Changed 6 years ago by
Keywords: | review removed |
---|---|
Owner: | changed from argonemyth to Thijs Triemstra |
Status: | assigned → new |
Hey thijs,
Thanks for updating the mail component with FilePath
. I found two little things might take you 5 minutes to correct.
- There is still a
file
object lingering around:
- You don't need to add "b" in
test_mail.RelayerTestCase
like antoine said in comment:3. As the file is always opened in binary mode if you useFilePath.open()
.
That's all, thanks you again!
comment:11 Changed 6 years ago by
Cc: | moijes12 added |
---|---|
Keywords: | review added |
Owner: | Thijs Triemstra deleted |
Patch was build against the branch.
comment:12 Changed 6 years ago by
Make that patch was created against the branch branches/mail-filepath-5809.
comment:13 Changed 6 years ago by
Keywords: | review removed |
---|---|
Owner: | set to moijes12 |
Thanks. The change still seems incomplete though. FilePath
should be used, not open
. This will also fix a related problem, which is that open(...).read()
relies on the garbage collector to close the file, which takes a while on PyPy and generates a warning on Python 3.
Changed 6 years ago by
Attachment: | 5809.2.patch added |
---|
comment:14 Changed 6 years ago by
Keywords: | review added |
---|---|
Owner: | moijes12 deleted |
comment:15 Changed 5 years ago by
Keywords: | review removed |
---|---|
Resolution: | → wontfix |
Status: | new → closed |
Following Plan/Python3 I'm closing this ticket. Sorry for the inconvenience.
When using pickle.dump, you want to use open mode "wb", not "w". Otherwise, looks good.