Allow gzip compression in high-level server response interface

[danielnelson]

Add encoding kwarg to enable_compression to select gzip or deflate and
set Content-Encoding header.

This is for issue #396.

I have a few concerns about this code:

I think that when using a high level interface one would generally want to simply enable all types of compression that are supported by the library, with selection automatically made based on the Accept-Encoding header. Potentially one may want to optionally specify a subset of the available encoding types, such as if we had compress support, maybe one would want to allow gzip and compress but not deflate.

I considered changing the signature to accept a sequence of encodings, however if we do this it is no longer intuitive to me how the force argument behaves.

Let me know how you think it would best to proceed.

[asvetlov]

Please fix pep8 errors

[asvetlov]

Maybe force parameter should accept False, True (synonym for 'deflate'), 'deflate' and 'gzip'?
Does it make sense?

[fafhrd91]

Enum with 3 values?

[asvetlov]

@fafhrd91 I believe enum is the perfect solution but we still should support True and False without breaking backward compatibility.
Also IIRC enum library is not available in Python 3.3. Would you add https://pythonhosted.org/flufl.enum/ as requered dependency?

[fafhrd91]

Let's drop py3.3 support

[ludovic-gasc]

Let's drop py3.3 support

+1

[asvetlov]

@fafhrd91 you are moving too fast from my perspective.
I'd like to support 3.3 until 3.5 release at least.

Anyway, how enum may process True and False? That's required for aiohttp backward compatibility.
Do you suggest accepting both bool and enum for force parameter? That makes sense.

[fafhrd91]

I prefer to use enum. Is flufl.enum compatible with py3.4 enums?
I am fine with enum dependency

[asvetlov]

It's fully compatible and it is the base library for standard enum.

[fafhrd91]

Lets then just use enum

[danielnelson]

Okay so based on this discussion here is a rough draft of what the new documentation might look like:

.. method:: enable_compression(force=None, *, encodings=None)

Enable compression.

When *force* is unset (default) compression encoding is
selected based on the :class:`TransferEncoding` in *encodings*
and the *Accept-Encoding* request header.  The default encodings
are :class:`TransferEncoding.deflate` and
:class:`TransferEncoding.gzip`.

When *force* is set to a :class:`TransferEncoding`, this encoding
will be used and *Accept-Encoding* is not checked.

If *force* is set to ``True``, :class:`TransferEncoding.deflate`
will be used.

[asvetlov]

@danielnelson did you mean .. method:: enable_compression(force=None) ?

[danielnelson]

I'm not sure I understand. Are you saying you don't expect a encodings kwarg or are you asking about the default value of force?

[asvetlov]

I say encoding is redundant and force should accept True/False for backward compatibility and TransferEncoding for actual encoding specification. False is alias for TransferEncoding.plain, True for TransferEncoding.deflate.

[fafhrd91]

I wouldn't worry about backward compatibility here.

[asvetlov]

Sorry, but I do care about backward compatibility.
Maybe the compromise is:

1. introduce compress property and enable_compress(transfer_encoding: TransferEncoding) method.
2. Deprecate compression and enable_compression().
3. Call new API from deprecated one.

[asvetlov]

content_coding is the term used by RFC 2616, we can use this name.
But maybe it's sounds a bit confusing.

[asvetlov]

compress is a verb but content_coding is a noun.

[danielnelson]

Anywhere I said transfer encoding above should definitely be content coding, and of course the header is Content-Encoding.

Having backwards compatibility here wouldn't introduce too much cruft, and I also think we are not completely sure what we would need from a compress interface. So I would prefer to just add backwards compatibility for now.

[asvetlov]

Please use str values: deflate = 'deflate'

[kxepal]

Also would like to note that this list of encodings is not complete. Never saw compress in the wild, however, but identity is pretty common one.

[danielnelson]

I believe this is the current list of registered codings. I'll add a comment about how this is enum only represents our supported coding types.

[asvetlov]

Good finding! Please add a comment with this link

[danielnelson]

I have updated the pull request with the suggested changes.

[asvetlov]

LGTM

[asvetlov]

Merged. @danielnelson thanks for patience :)

[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.