apache / incubator-ponymail-foal

Apache Pony Mail Foal (Next Generation Suite)
https://ponymail.apache.org
Apache License 2.0
28 stars 14 forks source link

tools/import-mbox.py: fix Maildir get_file() #262

Closed t-lo closed 4 months ago

t-lo commented 4 months ago

Ran into issues recently when trying to use the mailbox import feature - for a Maildir mailbox in my case but I don't think that matters, mbox is likely affected, too.

The Python mailbox module's get_file() function only takes a single argument (two when counting the object's "self" reference), see https://docs.python.org/3/library/mailbox.html#mailbox.Maildir.get_file.

This simple one-line change change removes the spurious ...,True) argument to un-break mailbox import.

sebbASF commented 4 months ago

AFAICT the mbox version of get_file does have the extra parameter

sebbASF commented 4 months ago

See: https://github.com/python/cpython/blob/main/Lib/mailbox.py#L858

The bug is in the documentation.

t-lo commented 4 months ago

@sebbASF This makes the code break when importing Maildir because Maildir doesn't: https://github.com/python/cpython/blob/main/Lib/mailbox.py#L1125 So the call in import-mailbox.py needs to branch depending on whether it handles Maildir or mbox.

sebbASF commented 4 months ago

OK. Can you provide an updated PR?

t-lo commented 4 months ago

Looking into it right now :)

t-lo commented 4 months ago

Updated the PR. Since only mbox supports the additional kv parameter I've used a simple branch between mbox and "everything else". Tested with mbox and Maildir imports; both appear to work.

sebbASF commented 4 months ago

Thanks! Although your patch works, I decided to use the existing maildir boolean for consistency.

t-lo commented 4 months ago

@sebbASF No problem, happy that it's fixed now.