Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

accept-encoding.acceptable_offers is not very useful for the common use case #387

Open
mmerickel opened this issue Oct 29, 2018 · 4 comments

Comments

@mmerickel
Copy link
Member

While this api is compliant with the RFC at https://tools.ietf.org/html/rfc7231#section-5.3.4, it is not what anybody really expects or wants in the case of a missing header. The answer is pretty much always to use identity when a header is missing. This is what the deprecated best_match api used to do, with best_match(['identity', 'gzip'], default_match='identity').

>>> create_accept_encoding_header(None).acceptable_offers(['identity', 'gzip'])
[('identity', 1.0), ('gzip', 1.0)]
>>> create_accept_encoding_header(None).best_match(['identity', 'gzip'], default_match='identity')
'identity'

The proposed api would be something that implements the following:

offers = ['gzip', 'identity']  # order matters here
default_encoding = 'identity'
if request.accept_encoding:
    encodings = request.accept_encoding.acceptable_offers(offers)
    target_encoding = encodings[0][0] if encodings else default_encoding
else:
    target_encoding = default_encoding

This is very similar to best_match but without any weird server-side quality multiplier logic.

@mmerickel
Copy link
Member Author

@whiteroses do you have any thoughts about this?

@whiteroses
Copy link
Contributor

@mmerickel Sorry, I'm not quite understanding the issue? Why is 'identity' in both offers and default_match/default_encoding? And

>>> create_accept_encoding_header(None).best_match(['identity', 'gzip'], default_match='identity')
'identity'

is not actually using the default_match:

>>> create_accept_encoding_header(None).best_match(['identity', 'gzip'], default_match='default_match')
'identity'

From WebOb 1.7.4, the original NoAccept.best_match:

>>> NoAccept().best_match(offers=['identity', 'gzip'], default_match='default_match')
'identity'
>>> NoAccept().best_match(offers=['gzip', 'identity'], default_match='default_match')
'gzip'

For comparison:

>>> create_accept_encoding_header(None).best_match(['identity', 'gzip'], default_match='default_match')
'identity'
>>> create_accept_encoding_header(None).best_match(['gzip', 'identity'], default_match='default_match')
'gzip'

and

>>> create_accept_encoding_header(None).acceptable_offers(['identity', 'gzip'])
[('identity', 1.0), ('gzip', 1.0)]
>>> create_accept_encoding_header(None).acceptable_offers(['gzip', 'identity'])
[('gzip', 1.0), ('identity', 1.0)]

acceptable_offers is returning the same offer in the first element of the list as best_match did, isn't it? None of them use the default_match?

@mmerickel
Copy link
Member Author

@whiteroses I don't want to get too bogged down in the details of how best_match used to work. I get that there's things going on there. The focus of my issue is that the common use case for accept-encoding is the snippet I described at the bottom of my issue and it'd be nice if we could come up with a way to provide an api around it. The RFC states that any encoding is acceptable when there is no header provided, but in practice literally no one wants that - they want a default (of likely 'identity') when a header is not provided.

@whiteroses
Copy link
Contributor

@mmerickel Was just reassuring you that this was not handled by the deprecated best_match api, so nothing was lost. I think I understand what you are looking for now: I can make a PR with a .best_response_coding method that does the right thing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants