python-hyper / python-hyper/h2

Consider further adjustments to the headers structure.

Ouverte
#205 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

Langage dominant
Python
Étoiles
1k
Forks
187
Métriques de merge des PR
Aucune PR mergée en 30 j

Description

@rbtcollins and I have had a really interesting discussion on IRC about #194, and in particular about the API decision that was made in that issue. For those who don't remember, the end result was that we have a tuple-like datastructure (actually a tuple-subclass) that has a flag on it to indicate whether a header field should be indexable at the HPACK layer.

@rbtcollins has pointed out that this API is a bit prone to mis-use. In particular, the HeaderTuple quacks very much like a tuple, right the way down to its equality comparison (NeverIndexedHeaderTuple('name', 'value') == ('name', 'value')). This runs the risk of users who incautiously use this API failing to notice the distinction, and worse, failing to preserve it when working with the header fields.

Now, this is mostly a concern for intermediaries. hyper-h2 does some automatic never-indexing for some fields, and in most cases for clients/servers that will be enough (and we'll likely add a new API for hooking into that shortly). However, it's vital that intermediaries preserve the relevant semantics of these header fields through their manipulations, and that is something that they could easily get wrong if they're insufficiently cautious.

So we should consider whether or not we can replace this with a better API. The "headers as list-of-tuples" API is a really good one, partly because it preserves all the relevant data about the header fields, but also because it makes header fields immutable data structures, which solves a whole lot of pain points (and indeed we should ask ourselves whether hyper-h2 should convert the header iterable to a tuple, instead of a list, when emitting it to the user on events). We want to preserve this as much as possible.

Three alternative ideas have been proposed, the first two of which of which are backward-incompatible with the current headers representation and would require a SemVer Major version bump.

  1. Some kind of composite data structure. Essentially, we'd change headers from a HeaderTuple to a dictionary: a bit like {'indexable': True, 'field': field}, where field is a HeaderTuple or tuple.
  2. Wrap the header tuple, rather than subclass. Essentially, have HeaderField = namedtuple('HeaderField', ['indexable', 'field']).
  3. Adjust the equality logic of HeaderTuple to prevent the NeverIndexedHeaderField from comparing equal to a regular tuple.

My thoughts:

  • If we believe this is a problem that requires an API change, I don't think (3) goes far enough. If this information is important enough to make users think about it, (3) doesn't do that: in particular, it makes it all-to-easy for users to accidentally throw this information away (by unpacking and repacking the tuple). It's basically only marginally better than the current state, and doesn't save users from their most likely mistake.
  • (1) loses some of our immutability wins: dicts are mutable, and that's pretty sad. Additionally, there's no dot-access. On the other hand, it's very easily extensible.
  • Of the set, (2) is probably my favourite.
  • Regardless of what we do, revising this API is a big change: we'll affect two functions (send_headers, push_stream) and five events (RequestReceived, ResponseReceived, InformationalResponseReceived, TrailersReceived, PushedStreamReceived), including two events that are in the basic set of events that all implementations will want to handle. Essentially, this means that if we do (1) or (2) we'll definitely break every single implementation that currently exists.

At this point I'd like to solicit some feedback from the community. Is this a problem that's big enough to justify a fairly substantial API revision? If so, do you have a preferred API design? If not, why not?

I'm going to CC: @mhils @Kriechi @hawkowl @sigmavirus24 and @jimcarreer as the community reps on this. Feel free to also CC anyone else who might have a useful opinion here.

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Commencez par examiner les fonctions send_headers et push_stream ainsi que les événements RequestReceived, ResponseReceived, InformationalResponseReceived, TrailersReceived et PushedStreamReceived mentionnés dans l’issue. Comparez les trois représentations d’en-têtes proposées et leur impact sur la rétrocompatibilité ; le travail est considéré comme terminé lorsque la communauté a sélectionné et spécifié une révision de l’API.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
python
Domaine
api, networking
Type d'issue
Refactorisation
Difficulté
5/5
Temps estimé
Plus d'une semaine
Activité
À l'abandon
Clarté
À clarifier
Accessibilité débutants
20/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.