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

fix(netx): ensure we create ~same HTTP3 and HTTP2 transports #795

Merged
merged 1 commit into from
Jun 5, 2022

Conversation

bassosimone
Copy link
Contributor

  1. Use the netxlite.NewHTTPTransport factory for creating a new
    HTTP2 (and HTTP1) transport;

  2. Recognize the netxlite.NewOOHTTPTransport has now become
    an implementation detail so make it private;

  3. Recognize that netxlite.NewHTTP3Transport should call
    netxlite.WrapTransport so it returns the same typechain
    returned by netxlite.NewHTTPTransport (modulo, of course,
    the real underlying transport), so ensure that we are
    calling netxlite.WrapTransport in NewHTTP3Transport;

  4. Recognize that the table based constructor inside of
    netx needs a logger to create HTTPTransport instances using
    either netxlite.NewHTTP{,3}Transport so pass this argument
    along and ensure it's not nil using a constructor inside
    model that guarantees that;

  5. Cleanup netx's tests to avoid type asserting on the
    typechains returned by netxlite since we already test
    that inside netxlite;

  6. Recognize that now we can make more legacy names inside
    of netxlite private because we don't need to use them
    inside tests anymore (because of previous point).

Reference issue: ooni/probe#2121

1. Use the netxlite.NewHTTPTransport factory for creating a new
HTTP2 (and HTTP1) transport;

2. Recognize the netxlite.NewOOHTTPTransport has now become
an implementation detail so make it private;

3. Recognize that netxlite.NewHTTP3Transport should call
netxlite.WrapTransport so it returns the same typechain
returned by netxlite.NewHTTPTransport (modulo, of course,
the real underlying transport), so ensure that we are
calling netxlite.WrapTransport in NewHTTP3Transport;

4. Recognize that the table based constructor inside of
netx needs a logger to create HTTPTransport instances using
either netxlite.NewHTTP{,3}Transport so pass this argument
along and ensure it's not nil using a constructor inside
model that guarantees that;

5. Cleanup netx's tests to avoid type asserting on the
typechains returned by netxlite since we already test
that inside netxlite;

6. Recognize that now we can make more legacy names inside
of netxlite private because we don't need to use them
inside tests anymore (because of previous point).

Reference issue: ooni/probe#2121
@bassosimone bassosimone requested a review from hellais as a code owner June 5, 2022 15:35
@bassosimone bassosimone merged commit c6b3889 into master Jun 5, 2022
@bassosimone bassosimone deleted the issue/2121 branch June 5, 2022 15:41
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

Successfully merging this pull request may close these issues.

1 participant