#twisted

Saman: hm, do you mean the fetch method of IMailboxIMAP implementers?

runciter, yes

the documentation does not even mention possibility of returning `deferred` yet `do_FETCH` expects that the result `maybeDeferred()`: https://github.com/twisted/twisted/blob/twisted...

I assume the return type can be => `Deferred<Iterable<IMessageIMAP>> | Iterable<IMessageIMAP>`

while I was wondering how to change the behavior to accept a return type of => `Iterable<Deffered<IMessageIMAP>>`

Saman: you're close - the return type is actually an iterable of (message sequence numbers, IMessageIMAP providers)

the documentation needs to be fixed :(

because you can indeed return a deferred from fetch

new core defect https://tm.tl/#9196 by altendky: Codecov report search disabled on AppVeyor

yeah, right, return is => `iterable<(message seq-num, IMessageIMAP)>` or `Deferred<iterable<(message seq-num, IMessageIMAP)>>`

Saman: so i believe the intent behind this code is that you should be able return a deferred that fires when you've gotten access to some mailbox, and that you should be able to lazily load sequence numbers and imap messages from that mailbox

I am wondering how to turn it into => `iterable<Deferred<(message seq-num, IMessageIMAP)>>`. cuz loading all the `message-set` into memory in a sigle deferred can quite expensive considering message sets can request lengthy sequences

ah, i see

one moment

Saman: i think the short answer is no

Saman: not without some non-trivial surgery to IMAP4Server

Saman: if you're willing to do that, we can file a bug and get a patch going

Saman: i've been working on porting imap4 to python 3, so i'm relatively familiar with this code and would be happy to do a review

I am def willing to implement it as this behavior is really troublesome working even with well known MUAs as they request large sequesnces in a single command ( more than 1000 messages to be loaded at the same time in memory)

Saman: imap4 has a lot of problems

that's the first question, and the second one is that, this `fetch` interface is to accommodate all variations IMAP FETCH commands, which in turn means that implementers have to load the entire message into memory although the command might only be asking for FALGS,UID or any other metadata

Saman: yeah

one workaround can be for `IMessagePart` method to be able to return deferred types, so to load the content on demand, and not for `getUID`/ `getFLAGS` methods

hm

Saman: so getBodyFile is expensive even if no reads occur, because it's not cheap to open a file?

`getBodyFile` is expensive now even if it doesn't get called, as `IMessagePart` interface expects the entire message body to be loaded at instantiation time of `IMessage`

where as bunch of IMAP commands will only result in calls to `getUID` or ... and not `getBodyFile`

whereas*

Saman: IMessageIMAPPart expects the entire message body to be loaded because of methods like getHeaders?

that is, the methods that return metadata about a message?

well, I store headers separately from the rest of MIME message, as entire MIME is very relative to few lines of headers

that's where I see the limitation of this expectation

ok

very large relative*

Saman: so you'd like all the IMessageIMAPPart methods (getHeaders, getBodyFile) to return deferreds?

not just getBodyFile?

they can all be maybeDeffered

or would it be sufficient to just allow getBodyFile to return a deferred?

I think that'd be more reasonable in terms of having an identical interface

sorry, i meant that all methods should allowed to return deferreds, not that they'll have to return deferreds

I image it'd be better for all of them to be allowed to return deferred

so it's up to the implementor to decide

depending on their design choices

hm

Saman: it might be easier to allow fetch to return an _iterable_ of deferreds, as you suggested

and then allow getBodyFile to _also_ return a deferred

this would be not unlike an HTTP request, in which the metadata is available before the request body

would that be ok?

so the only part of IMessageIMAPPart that would change would be that getBodyFile could return a deferred, and then the only part of IMailboxIMAP that would change is that the iterable could consist of two-tuples or deferreds that fire with two-tuples

yeah, that will be a huge improvement, yet having the rest of the methods returning deferreds wouldn't be troublesome

1) having `fetch` return `<iterable<deferred<(..,..)>>` solves inefficient memory usage and sequential writes to transport (instead on humongous batch of results) 2) `IMessagePart` method returning deferreds will avoid loading of the entire message while the request is only for light weight metadata (header, flags, uid, internal_date).

Saman: i think the hardest part will be allowing getBodyFile to return a Deferred

the next most difficult thing would be to allow all the methods to return deferreds

it's really useful to allow getBodyFile to return a deferred so i see no way around the most difficult thing

but i'd like to avoid the second most difficult thing

i see your point, side question, are you an implementor of the module ?

the current implementation

Saman: so i spent some time porting it, or trying to port it, to python 3

Saman: https://github.com/twisted/twisted/pull/805

Saman: so i am relatively familiar with the code

Saman: by the way, if you have the time, i'd really appreciate it if you could try your code with that branch

even if it's running on python 2 - it'd be great to verify i didn't break anything for anybody

gotcha, I would have, but I am using a custom python 2.7 blob

ok

I can do that

thanks!!

Saman: i'm going to file a ticket that describes your problem

ok, that's great, we I can follow up on github, my github id is : `samanrtehrani`

Saman: cool!!

Saman: if there's any trouble with the branch in pull request 805, feel free to comment on github

so, do you know any alternative implementations of imap4 in python ?

Saman: none that are very good

do you use twisted in production ?

Saman: i have many times, yes

but never its IMAP4Server

i've only used IMAP4Server for fun

yeah, I mean for IMAP4Server. the reason Im asking is that I want to use it in production, and options so far are 1) implement the server from scratch (which is PITA ofcourse) 2) deal with the current shortcomings and patch them 3) find a better performant alternative opensource implementation

Saman: indeed

Saman: the best that i can offer is that imap4 is under active development

in anycase, I am definitely willing to patch twisted, so would appreciate if you'd file the issue

hm

maybe we should have a new interface

IMailboxIMAPDeferred or something

that would allow us to leave the existing code path alone

well the caller of that method `spew_${}` methods: https://github.com/twisted/twisted/blob/twisted...

the real surgery has to happen in there

new core enhancement https://tm.tl/#9197 by markrwilliams: IMailboxIMAP.fetch eagerly loads entire messages

Saman: ^

Saman: so my thinking is that, for example, here: https://github.com/twisted/twisted/blob/trunk/s...

IMAP4Server could do: if IMailboxAsynchronousIMAP.providedBy(self.mailbox): # handle deferreds

else: # call fetch, and __cbFetch, etc

kenaan hi, thanks for the issue

is kenaan a bot :D

Yes.

it's written in twisted!

we believe in our code

Saman: the advantage of defining this new interface is that we don't have to touch the existing interface, so there's no chance of breaking it

Saman: and IMAP4Server can ask if the mailbox instance has opted into using Deferreds

so IMAP4Server's code for the existing interface can also stay the same

I don't mind the idea of new interface. so the as mentioned up, I think there are 2 problems in hand : 1) having `fetch` return `<iterable<deferred<(..,..)>>` solves inefficient memory usage and sequential writes to transport (instead on humongous batch of results) 2) `IMessagePart` method returning deferreds will avoid loading of the entire message while the request is only for light weight metadata (header, flags, uid, internal_date

the first one I suppose can be fixed using a DeferredList

at the point of the code you just sent

basically iterating on the returned iterator from `fetch` and adding callbacks to each of the returned deferreds

Saman: ah, so for the second point - IMailboxAsynchronousIMAP.fetch will give back IMessageAsynchronousIMAP, whose getBodyFile method will return Deferreds

Saman: that's the other advantage of using a new interface - the code can assume that mailboxes that provide IMailboxAsynchronousIMAP *always* return Deferreds from getBodyFile

I agree w hacving a new interface, I think the conditional logic you are bringing up should just happen in `__cbFetch`

if the returned tuple is instance of (number, IMailboxAsyncIMAP), then do another way of spewing messages

here: https://github.com/twisted/twisted/blob/trunk/s...

the returned value of fetch can be either `iterable<deferred<(..,..)>>` or `deferred<iterable<(..,..)>>`

Saman: hm, i was thinking that IMailboxAsyncIMAP.fetch wouldn't return a deferred ever - just an iterable of deferreds

but now i'm not so sure

Saman: my reasoning is that IMailboxIMAP.fetch can return a deferred because it has to load a bunch of messages

Saman: but IMailboxAsyncIMAP won't have to load a bunch of messages, so it can return an iterator (really, a generator) that yields deferreds representing each message that can be loaded

yeah, that sounds reasonable as well, I am not 100% sure whats the best way , need to think about it more to digest all the aspects

Saman: no problem :) if you have time to work on this, take whatever approach seems the best to you

Saman: we can work out the details later

are the issues added to github automatically ?

they are not

the way it works is: you file a ticket on trac (https://tm.tl/#9197 here)

then, when you have some code, you open a PR on github, and paste the link to the ticket (https://tm.tl/#9197) into the PR

then you go back to trac (https://tm.tl/#9197) and add the "review" keyword

gotcha

how frequent is the usually the review process?

and who is in the release manager in charge?

hawkowl has been managing releases for a long time now and she's great at it

Saman: anybody can do a review

you don't even have to be a twisted committer

Tickets pending review: https://tm.tl/#8966 (the0id), #9101, #9118 (the0id), #4964 (jameshilliard), #9131, #9138, #9143, #7930 (wsanchez)

runciter: yeah, ok , thanks for all the info, I actually did shoot an email to hawkowl before joining this channel. I will look into the possibilities and share more info later

Saman: cool!

Saman: it's probable that we'll merge the python 3 stuff pretty soon

after that i can dedicate time to 9197 :)

sounds great