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

Feature client transport config #40

Merged

Conversation

FriMeDev
Copy link
Contributor

@FriMeDev FriMeDev commented Nov 4, 2023

What kind of change does this PR introduce?

Add the possibility to define custom transport settings for the http.Client. Furthermore the renaming prevents stuttering (former Client.clientTransport)

What is the current behavior?

Any information can be found in #39

What is the new behavior?

It is now possible to define custom transport behavior by setting Client.Transport.Parent:

func main() {
	c := postgrest.NewClient("localhost:3000", "", nil)

	c.Transport.Parent = &http.Transport{
		TLSClientConfig: &tls.Config{
			RootCAs:            x509.NewCertPool(),
			InsecureSkipVerify: false,
		},
	}
}

Additional information

Due to the behavior of httpmock used for testing, the implemented RoundTrip() is not as clean as it could be. For testing with httpmock to work properly http.DefaultTransport.RoundTrip() must be called in RoundTrip() in client.go.

change type of Client.Transport to pointer
change add exposed Parent to transport
change use Parent in RoundTripper
needed to provide compatibility with httpmock for testing.
@tranhoangvuit
Copy link
Contributor

Hi @Fritte795, it looks good to me, but could you please resolve the conflict first?

@muratmirgun
Copy link
Member

@tranhoangvuit I will fix this conflicts tonight

@muratmirgun muratmirgun merged commit 907a6dd into supabase-community:main Jan 6, 2024
1 check passed
@elijahmorg
Copy link
Contributor

This PR clobbered the changes made in #41

In current main you cannot set apikey and token separately.

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.

4 participants