Opened 2 years ago

Closed 2 years ago

#5809 enhancement closed wontfix (wontfix)

replace usage of file with FilePath in twisted.mail

Reported by: thijs Owned by:
Priority: low Milestone: Python-3.x
Component: mail Keywords: easy
Cc: thijs, moijes12@… Branch: branches/mail-filepath-5809
(diff, github, buildbot, log)
Author: thijs Launchpad Bug:

Description (last modified by thijs)

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)

mail-open-5809.patch (5.4 KB) - added by thijs 2 years ago.
5809.patch (922 bytes) - added by moijes12 2 years ago.
tried to add argonemyth's suggestions
5809.2.patch (933 bytes) - added by moijes12 2 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 2 years ago by thijs

  • Description modified (diff)

Changed 2 years ago by thijs

comment:2 Changed 2 years ago by thijs

  • Keywords review added

comment:3 Changed 2 years ago by antoine

  • Keywords review removed
  • Owner set to thijs

When using pickle.dump, you want to use open mode "wb", not "w". Otherwise, looks good.

comment:4 Changed 2 years ago by thijs

  • Description modified (diff)
  • Status changed from new to assigned
  • Summary changed from replace usage of file with open in twisted.mail to replace usage of file with FilePath in twisted.mail

comment:5 Changed 2 years ago by thijs

  • Author set to thijs
  • Branch set to branches/mail-filepath-5809

(In [35027]) Branching to 'mail-filepath-5809'

comment:6 Changed 2 years ago by thijs

(In [35029]) replace the usage of file with FilePath in twisted.mail, refs #5809

comment:7 Changed 2 years ago by thijs

  • Keywords review added
  • Owner thijs deleted
  • Status changed from assigned to new

comment:9 Changed 2 years ago by argonemyth

  • Owner set to argonemyth
  • Status changed from new to assigned

comment:10 Changed 2 years ago by argonemyth

  • Keywords review removed
  • Owner changed from argonemyth to thijs
  • Status changed from assigned to new

Hey thijs,

Thanks for updating the mail component with FilePath. I found two little things might take you 5 minutes to correct.

  1. There is still a file object lingering around:

http://twistedmatrix.com/trac/browser/branches/mail-filepath-5809/twisted/mail/test/test_mail.py#L202

  1. 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 use FilePath.open().

That's all, thanks you again!

Changed 2 years ago by moijes12

tried to add argonemyth's suggestions

comment:11 Changed 2 years ago by moijes12

  • Cc moijes12@… added
  • Keywords review added
  • Owner thijs deleted

Patch was build against the branch.

comment:12 Changed 2 years ago by moijes12

Make that patch was created against the branch branches/mail-filepath-5809.

comment:13 Changed 2 years ago by exarkun

  • 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 2 years ago by moijes12

comment:14 Changed 2 years ago by moijes12

  • Keywords review added
  • Owner moijes12 deleted

comment:15 Changed 2 years ago by therve

  • Keywords review removed
  • Resolution set to wontfix
  • Status changed from new to closed

Following Plan/Python3 I'm closing this ticket. Sorry for the inconvenience.

Note: See TracTickets for help on using tickets.