Skip to content
This repository has been archived by the owner on Jun 6, 2023. It is now read-only.

Function for customizing http.Transport for a http/2 client #87

Closed
cubicdaiya opened this issue Sep 28, 2016 · 2 comments
Closed

Function for customizing http.Transport for a http/2 client #87

cubicdaiya opened this issue Sep 28, 2016 · 2 comments
Labels

Comments

@cubicdaiya
Copy link

Now buford provides the function of push.NewClient(cert tls.Certifcate) (*http.Client, error) for creating a http/2 client.

client, err := push.NewClient(certificate)

But push.NewClient() can not customize http.Transport like the code below for a http/2 client.

// example definition
transport := &http.Transport{
        TLSClientConfig:     config,
        MaxIdleConnsPerHost: 16,
}

MaxIdleConnsPerHost is important for sending many notifications to APNs once. Accoding to the document below (Best Practices for Managing Connections), increasing connections to APNs seems to be recommended for improving performance. (MaxIdleConnsPerHost is 2 by default. )

https://developer.apple.com/library/content/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/Chapters/APNsProviderAPI.html#//apple_ref/doc/uid/TP40008194-CH101-SW1

Though it is good if there is the function for customizing http.Transport for a http/2 client, what do you think?

@nathany
Copy link
Contributor

nathany commented Sep 28, 2016

Thanks for mentioning the MaxIdleConnsPerHost consideration.

I'm not sure that it's really necessary to add a function for Buford to customize transport, or what it would look like if I did. You can easily copy/paste and modify these 10 lines of code to create your own HTTP client that you pass to push.NewService.

// NewClient sets up an HTTP/2 client for a certificate.
func NewClient(cert tls.Certificate) (*http.Client, error) {
    config := &tls.Config{
        Certificates: []tls.Certificate{cert},
    }
    config.BuildNameToCertificate()
    transport := &http.Transport{TLSClientConfig: config}

    if err := http2.ConfigureTransport(transport); err != nil {
        return nil, err
    }

    return &http.Client{Transport: transport}, nil
}

(from https://github.com/RobotsAndPencils/buford/blob/master/push/service.go#L43)

FYI, the NewClient function and certificate loading may eventually all go away with #63, though not in the immediate future.

@cubicdaiya
Copy link
Author

Thank you for the reply and suggestion.

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

No branches or pull requests

2 participants