-
Notifications
You must be signed in to change notification settings - Fork 845
Golang driver replace websockets with http #3059
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
Conversation
…s no longer applicable to tinkerpop 4.0.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3059 +/- ##
============================================
+ Coverage 77.87% 77.99% +0.11%
- Complexity 13578 13908 +330
============================================
Files 1015 1019 +4
Lines 59308 59933 +625
Branches 6835 6950 +115
============================================
+ Hits 46184 46742 +558
- Misses 10817 10858 +41
- Partials 2307 2333 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gremlin-go/driver/httpTransporter.go
Outdated
| if transporter.connSettings.enableUserAgentOnConnect { | ||
| req.Header.Set(userAgentHeader, userAgent) | ||
| } | ||
| if transporter.connSettings.enableCompression { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enableCompression existed in 3.7 to manage per message deflate in WebSockets, but was removed in master. Maybe this is a setting that also applies to master but I feel like in the vast majority of cases this should just be left on.
gremlin-go/driver/httpProtocol.go
Outdated
|
|
||
| // receives a binary response message, deserializes, and adds results to the ResultSet | ||
| func (protocol *httpProtocol) receive(rs ResultSet, msg []byte) error { | ||
| resp, err := protocol.serializer.deserializeMessage(msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all messages return as GraphBinary. It's possible for some errors to still be "application/json". The driver needs to decide what to do by reading the content-type header.
… connection_test.go.
…r to use lowercase.
|
VOTE +1 |
|
|
||
| func (transporter *httpTransporter) createResponseError(respBytes []byte, resp *http.Response) error { | ||
| contentType := resp.Header.Get(contentTypeHeader) | ||
| if contentType == "application/json" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very small nit: are there no constants for commonly used HTTP values?
|
VOTE +1 |
1 similar comment
|
VOTE +1 |
Replaced web sockets with http for Golang driver and removed a lot of code that is no longer applicable for Tinkerpop 4.0. Note that tests are not yet enabled as they will not pass without changes to serialization.
The following was changed as part of conversion from web sockets to http:
The following was removed as no longer applicable for Tinkerpop 4.0:
Out of scope for future PRs: