Ticket #5809 enhancement closed wontfix

Opened 10 months ago

Last modified 6 months ago

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
Author: thijs Launchpad Bug:

Description (last modified by thijs) (diff)

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

mail-open-5809.patch Download (5.4 KB) - added by thijs 10 months ago.
5809.patch Download (0.9 KB) - added by moijes12 8 months ago.
tried to add argonemyth's suggestions
5809.2.patch Download (0.9 KB) - added by moijes12 8 months ago.

Change History

1

Changed 10 months ago by thijs

  • description modified (diff)

Changed 10 months ago by thijs

2

Changed 10 months ago by thijs

  • keywords review added

3

Changed 10 months ago by antoine

  • owner set to thijs
  • keywords review removed

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

4

Changed 10 months ago by thijs

  • status changed from new to assigned
  • description modified (diff)
  • summary changed from replace usage of file with open in twisted.mail to replace usage of file with FilePath in twisted.mail

5

Changed 10 months ago by thijs

  • branch set to branches/mail-filepath-5809
  • branch_author set to thijs

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

6

Changed 10 months ago by thijs

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

7

Changed 10 months ago by thijs

  • owner thijs deleted
  • status changed from assigned to new
  • keywords easy, review added; easy removed

8

Changed 10 months ago by thijs

9

Changed 9 months ago by argonemyth

  • status changed from new to assigned
  • owner set to argonemyth

10

Changed 9 months ago by argonemyth

  • keywords easy added; easy, 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

2. 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 8 months ago by moijes12

tried to add argonemyth's suggestions

11

Changed 8 months ago by moijes12

  • owner thijs deleted
  • keywords review added
  • cc moijes12@… added

Patch was build against the branch.

12

Changed 8 months ago by moijes12

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

13

Changed 8 months 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 8 months ago by moijes12

14

Changed 8 months ago by moijes12

  • keywords review added
  • owner moijes12 deleted

15

Changed 6 months ago by therve

  • status changed from new to closed
  • keywords review removed
  • resolution set to wontfix

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

Note: See TracTickets for help on using tickets.