Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve overall quality of twisted.mail.pop3.pop3 #9120

Open
twisted-trac opened this issue Apr 25, 2017 · 6 comments · May be fixed by #11898
Open

Improve overall quality of twisted.mail.pop3.pop3 #9120

twisted-trac opened this issue Apr 25, 2017 · 6 comments · May be fixed by #11898

Comments

@twisted-trac
Copy link

evilham's avatar @evilham reported
Trac ID trac#9120
Type enhancement
Created 2017-04-25 12:54:37Z

While fixing #9100, I detected a few issues with this module relating:

  • Python3 support
  • Coding standards
  • Coverage

I would like to work on this as a way of getting more comfortable with Twisted's development; if everything goes well, I could move afterwards to other modules in twisted.mail that need some love:
https://twistedmatrix.com/trac/query?status=assigned&status=new&status=reopened&component=mail&order=priority

Searchable metadata
trac-id__9120 9120
type__enhancement enhancement
reporter__evilham evilham
priority__low low
milestone__None None
branch__ 
branch_author__ 
status__new new
resolution__None None
component__mail mail
keywords__None None
time__1493124877799937 1493124877799937
changetime__1493124877799937 1493124877799937
version__None None
owner__None None

@Zectbumo
Copy link

Zectbumo commented Jun 5, 2023

Completing this enhancement will probably also solve #1663

@Zectbumo
Copy link

Zectbumo commented Jun 5, 2023

possible duplicate of #878?

@adiroiban
Copy link
Member

Well PR #878 was fixing #9269

As long as the issue is not fixed this can't be a duplicate.

To, this doesn't look like a duplicate, but as a followup ticket.

I guess that the py3 migration was just to make sure the current automated tests are green... but nothing more.

And we have this issue, so that now that we have a py3 base, someone who wan't to use POP3 with Twisted can continue to work and fix any other issues.

Cheers

@Zectbumo
Copy link

Zectbumo commented Jun 5, 2023

I have made some edits to make pop3 compatible with python3. I am happy with all of them except I didn't know what to do with tap. The CLI twistd mail command uses str but everything internally is bytes. pop3 can now be used in python3 if one manually set tap options with byte options using Zectbumo:twisted:trunk. I can do a PR now or later if I can get a recommendation on a good solution as to what to do with CLI tap using strings to get it working.
this still doesn't work: twistd mail --no-smtp --maildirdbmdomain=example.com=/tmp/example.com --user=joe=password
but this tac does now:

from twisted.mail import tap
from twisted.application import service

o = tap.Options()
o.parseOptions([
'--no-smtp',
'--maildirdbmdomain', b'example.com=/tmp/example.com',
'--user', b'joe=password',
])
o.postOptions()
mailService = tap.makeService(o)
application = service.Application('pop3')
mailService.setServiceParent(application)

@adiroiban
Copy link
Member

The CLI twistd mail command uses str but everything internally is bytes. pop3 can now be used in python3 if one manually set tap options with byte options

I am not using pop3 or imap support in Twisted.
So this is up to you.

I would say it would make sense for code in POP3 to also allow str at the high level... but I guess that when the py3 migration was done, the decision was to go with bytes since this is the most flexible option.

So for backward compatibility still allow bytes ... but also handle the case when username, password or mailboxes are passed as string.

@adiroiban
Copy link
Member

I have also never used the Twisted Application framework (TAC) files so I don't have experience with how people are expecting to interact with Twisted via a .tac file.

I think that at this point POP3 is not actively maintained in Twisted, so if you want to suggest a change, anything is ok, as long as the tests pass and the backward compatibility policy is followed.

if thigs are broken and POP3 is not usable in py3 we don't even have to worry about backward compatibility, and just fix it as you think is best.

Thanks for help with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants