fix(vertex): do not override custom http client when using vertex#238
fix(vertex): do not override custom http client when using vertex#238andreynering wants to merge 3 commits intoanthropics:mainfrom
Conversation
vertex/vertex.go
Outdated
| sdkoption.WithBaseURL(baseURL), | ||
| sdkoption.WithMiddleware(middleware), | ||
| sdkoption.WithHTTPClient(client), | ||
| sdkoption.WithHTTPClient(&http.Client{Transport: transport}), |
There was a problem hiding this comment.
We need a custom HTTP client here, but this package was essetially overriding it, making our custom client useless.
Here, I'm trying to combine both transports, but I'm sure yet if this is the best way to accomplish it. Seems to have worked well for us, though.
TODO: Handle scenario where Done!rc.HTTPClient is nil. (Probably fallback to the previous code).
| default: | ||
| return nil, fmt.Errorf("vertex middleware does not support %s %s", r.Method, r.URL.Path) |
There was a problem hiding this comment.
Looks like if we miss any path, we probably forgot something in the code. A good idea to error here?
andreynering
force-pushed
the
vertex-fixes
branch
from
57c96d3 to
5ee2923
Compare
andreynering
force-pushed
the
vertex-fixes
branch
2 times, most recently
from
7419bf6 to
7b952cd
Compare
andreynering
changed the title
andreynering
force-pushed
the
vertex-fixes
branch
from
7b952cd to
7b1c366
Compare
|
@gcemaj I wonder if you can take a look at this when possible? We're using a fork in the meantime because we need this. Thanks! |
andreynering
force-pushed
the
vertex-fixes
branch
from
7b1c366 to
00daa31
Compare
andreynering
force-pushed
the
vertex-fixes
branch
from
00daa31 to
62e9c54
Compare
|
I just rebased this. This is the last piece that, if merged, would allow us to use the package directly on Fantasy and Crush without having to maintain a fork. Let me know if it's not clear why we need this. |
|
@gcemaj @nikblanchet It would be awesome to get this merged! 🙏 I'm happy to answer any questions. We'd been using a fork instead of upstream just because of this small fix. |
1 participant
We've been using the
go-vcrlibrary to handle HTTP requests inside tests, and it gives you a custom HTTP transport, but the SDK also sets one. This PR combine both transports so they work together!