New timeout handling for commands

[cyberlis]

Right now we have timeout handling problem. When we are getting big email (tested on 27mb), downloading of email stops because of TimeoutError.

 @asyncio.coroutine
    def fetch(self, message_set, message_parts):
        return (yield from asyncio.wait_for(self.protocol.fetch(message_set, message_parts), self.timeout))

There is a problem here, because we have general timeout for the command. But we need timeout on recived data for command. Every time we are getting next chunk of data for fetch command, we have to reset timeout.

That's why I changed timeout handling. I added UpdatableTimeout class. It accepts a timeout parametr in constructor. When you call wait method of UpdatableTimeout timeout timer starts. You can call update method of UpdatableTimeout to restart timeout timer.

I added UpdatableTimeout to Command class. When you call wait method of Command it starts UpdatableTimeout timeout timer. Every time when we are getting new chunk of data for our current command (begin_literal_data, end_literal_data, append_literal_data, append_to_resp) I update UpdatableTimeout timer. self.new_data_timeout.update()

My code pass all tests

[coveralls]

Coverage decreased (-0.3%) to 93.686% when pulling 0910d110a0696cdca600835de2c99ec2ac84fa35 on cyberlis:master into 43f2088e4ae4a7109419c051e46cb8201ae083e7 on bamthomas:master.

[coveralls]

Coverage decreased (-0.3%) to 93.686% when pulling a5ddcd02b3cbf7b1023be125db79c96afee8d972 on cyberlis:master into 43f2088e4ae4a7109419c051e46cb8201ae083e7 on bamthomas:master.

[bamthomas]

Thank you for your proposal and sorry to answer this late.

So the intent if I understand well is to handle timeout at the Protocol level and not IMAP4 client, to avoid a global timeout with multiple client-server data exchanges (for fetch only I think).

Why wouldn't we set a timeout value in the ImapClientProtocol and trigger a timer at each data_received (canceling the previous one) ?

[cyberlis]

It is more convinient to have separate timer for each command. May be in the future you will have a necessity to set specific timeout timer for specific command. And to do that you will have to add many ifs to data_recived and this will make code ugly and hard to manage.

if we have only one timer in data_recived, we need to check if any command pending right now. If no command in progress, we have to remember to stop this timer.

And what if we have two pending commands. One command constantly gets new data and updates global timeout timer. But second command doesn't recive any data from server. Because of single timer, which updates by first command, we cant raise TimeoutError for first command.

I think my approach works correctly and it doesn't break API.

[bamthomas]

Yes I agree with you for the timeout per command. I pushed 3 commits : 2348e74 (adding t/o to command), cb8c60c and f425abb

Could you tell me if it is ok with your 27mb mail fetch ?

What was bothering me in this pull request was the side effect on UpdatableTimeout, I prefer redefining a new asyncio task and canceling the previous one. Besides we need in case of timeout to remove the command from the protocol state. Finally, we need also to test the timeout.

For now, I did not use the timeout in all commands, but just for fetch. If you want you can generalize it as you did in this pull request.

If it is ok, I will close this PR (if you don't mind) and release a new version. Thank you.

[cyberlis]

Yes. All works fine. Thank you