Get rid of email.Message, use MultiDict instead

[asvetlov]

In private conversation with @fafhrd91 we decided to get rid of using email.Message and switch to MultiDict for HTTP headers and so on.

email.Message handles HTTP headers well but pollutes public API with email specific methods which just doesn't works for aiohttp.

MultiDict implementation will be transplanted from aiorest library, which borrowed the class from WebOb with some cleanup.

The change will break backward compatibility, sorry, but it's strongly required to make aiohttp API clean and safe.

I hope the change will not make intolerable hurt for library users.

[ludovic-gasc]

+10 on this.
It was the first remark I've did myself when I've started to use aiohttp.
About Multidict implementation, I've found a lot in Python librairies, like in werkzeug, django, python-ldap...

[asvetlov]

@GMLudo please take a look on https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/multidict.py and feel free to blame if you will find the implementation doesn't satisfies your requirements.

[ludovic-gasc]

At first look, it's ok for me.
I will test when it will be implemented in aiohttp classes.

[fafhrd91]

@asvetlov do you work on this task?
I have some free time, I can integrate multidict.

[asvetlov]

@fafhrd91
I'll be busy for next two days, please feel free to move forward.

[fafhrd91]

@asvetlov Probably HttpResponse class should not inherit from http.message.HttpMessage class. We can add 'headers' attribute to response, but we need case insensitive multidict

[asvetlov]

@fafhrd91 do you really need for case insensitive one?
Can just keeping upper-case header names satisfy your requirements?

[asvetlov]

I can implement CIMultiDict and CIMutableMultidict next Monday. Sorry, tomorrow I'll probably not have enough free time.

[fafhrd91]

I'll implement. And we need immutable only

[asvetlov]

@fafhrd91 I agree with not inheriting HttpResponse from http.message.HttpMessage

[asvetlov]

ok

[asvetlov]

Also I worry a bit about several Request/Response classes.
We have Response and HttpResponse for example.
High-level server API will need for own Request/Response pair (see aiorest).
That's a mess. Probably good documentation can help.

Ideally I would like to rename:

• HttpRequest/HttpResponse to ClientRequest/ClientResponse
• Request/Response to RequestImpl/ResponseImpl (they are part of low-level API)
• Upcoming high-level pair should be ServerRequest and ServerResponse.

We can keep existing names as aliases to renamed classes.
Thoughts?

[fafhrd91]

I like ClientRequest/Response but I don't like RequestImpl.—
Sent from Mailbox

On Sat, Jun 7, 2014 at 12:12 PM, Andrew Svetlov notifications@github.com
wrote:

Also I worry a bit about several Request/Response classes.
We have Response and HttpResponse for example.
High-level server API will need for own Request/Response pair (see aiorest).
That's a mess. Probably good documentation can help.
Ideally I would like to rename:

• HttpRequest/HttpResponse to ClientRequest/ClientResponse
• Request/Response to RequestImpl/ResponseImpl (they are part of low-level API)

• Upcoming high-level pair should be ServerRequest and ServerResponse.
  We can keep existing names as aliases to renamed classes.
  Thoughts?

  Reply to this email directly or view it on GitHub:
  Get rid of email.Message, use MultiDict instead #62 (comment)

[asvetlov]

Choose something what you like -- but keep in mind that after adding high level server API we'll need to distinguish current low-level server request/response and upcoming high-level ones.

IIRC we have agreement that current server code will be saved (maybe with some minor changes) and we'll add high-level abstraction on top of current server code.

[fafhrd91]

fixed in @6d3050193d31a40e4236ad5e08f0a805aeb6fa06

[asvetlov]

I also would like to use MultiDict in server code.
@fafhrd91 do you have objections?

[fafhrd91]

-1 right now. i need to see specs and concerns of ServerRequest/ServerResponse code first. let’s work on this later.

On Jun 8, 2014, at 8:36 AM, Andrew Svetlov notifications@github.com wrote:

I also would like to use MultiDict in server code.
@fafhrd91 do you have objections?

—
Reply to this email directly or view it on GitHub.

[asvetlov]

Ok. Thus we are ready for 0.8 release?

[fafhrd91]

Mostly ready, I need to fix logging bug, then I'll make release.—
Sent from Mailbox

On Sun, Jun 8, 2014 at 9:48 AM, Andrew Svetlov notifications@github.com
wrote:

Ok. Thus we are ready for 0.8 release?

Reply to this email directly or view it on GitHub:
#62 (comment)

[fafhrd91]

i've decided to migrate to multidict while working on logging code

[asvetlov]

Looks like great change! Thank you.

[lock]

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.