Ticket #5809 enhancement closed wontfix

Opened 21 months ago

Last modified 17 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
(diff, github, buildbot, log)
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 21 months ago.
5809.patch Download (0.9 KB) - added by moijes12 19 months ago.
tried to add argonemyth's suggestions
5809.2.patch Download (0.9 KB) - added by moijes12 19 months ago.

Change History

1

Changed 21 months ago by thijs

  • description modified (diff)

Changed 21 months ago by thijs

2

Changed 21 months ago by thijs

  • keywords review added

3

Changed 21 months 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.

4

Changed 21 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 21 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 21 months ago by thijs

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

7

Changed 21 months ago by thijs

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

8

Changed 21 months ago by thijs

9

Changed 20 months ago by argonemyth

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

10

Changed 20 months ago by argonemyth

  • owner changed from argonemyth to thijs
  • keywords easy added; easy, review removed
  • 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 19 months ago by moijes12

tried to add argonemyth's suggestions

11

Changed 19 months ago by moijes12

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

Patch was build against the branch.

12

Changed 19 months ago by moijes12

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

13

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

14

Changed 19 months ago by moijes12

  • keywords review added
  • owner moijes12 deleted

15

Changed 17 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.