joao-r-reis commented on code in PR #1874:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1874#discussion_r2052186672
##########
cassandra_test.go:
##########
@@ -480,6 +480,12 @@ func TestCAS(t *testing.T) {
t.Fatalf("insert should have not been applied: title=%v
revID=%v modified=%v", titleCAS, revidCAS, modifiedCAS)
}
+ // TODO: This test failing with error due to "function dateof" on
cassandra side.
+ // It was officially removed in version 5.0.0. The recommended
replacement for dateOf is the toTimestamp function.
+ // As we are not testing against deprecated cassandra versions, it
makes sense to update tests to keep them up to date
Review Comment:
I believe this was done in #1828 so once that is merged and this PR is
rebased it should be fixed
##########
events.go:
##########
@@ -227,9 +227,6 @@ func (s *Session) handleNodeUp(eventIp net.IP, eventPort
int) {
return
}
- if d := host.Version().nodeUpDelay(); d > 0 {
Review Comment:
Why remove this?
##########
filters.go:
##########
@@ -24,39 +24,74 @@
package gocql
-import "fmt"
+import (
+ "fmt"
+ "net"
+)
// HostFilter interface is used when a host is discovered via server sent
events.
type HostFilter interface {
// Called when a new host is discovered, returning true will cause the
host
// to be added to the pools.
- Accept(host *HostInfo) bool
+ Accept(host Host) bool
+}
+
+// Host interface is provided to enable testing of custom implementations of
the HostFilter interface.
+type Host interface {
+ Peer() net.IP
+ ConnectAddress() net.IP
+ BroadcastAddress() net.IP
+ ListenAddress() net.IP
+ RPCAddress() net.IP
+ PreferredIP() net.IP
+ DataCenter() string
+ Rack() string
+ HostID() string
+ WorkLoad() string
+ Graph() bool
+ DSEVersion() string
+ Partitioner() string
+ ClusterName() string
+ Version() CassVersion
+ Tokens() []string
+ Port() int
+ IsUp() bool
+ String() string
+}
+
+// Since cassVersion is an unexported type, the CassVersion interface is
introduced
+// to allow better testability and increase test coverage.
+type CassVersion interface {
+ Set(v string) error
+ UnmarshalCQL(info TypeInfo, data []byte) error
+ AtLeast(major, minor, patch int) bool
+ String() string
Review Comment:
I think this needs some changes, just making an interface out of the methods
of the private type feels wrong here, we shouldn't expose `Set` or `Unmarshal`
method and we should definitely expose a way to get the actual version fields.
Ideally `CassVersion` would be:
```
type CassVersion interface {
Major() int
Minor() int
Patch() int
Qualifier() string
String() string
}
```
The methods on `cassVersion` can be converted into global private functions.
```
func (c *cassVersion) Set(v string) error
func (c *cassVersion) UnmarshalCQL(info TypeInfo, data []byte) error
func (c *cassVersion) unmarshal(data []byte) error {
func (c cassVersion) Before(major, minor, patch int) bool
func (c cassVersion) AtLeast(major, minor, patch int) bool
```
```
func parseCassVersion(v string) (CassVersion, error) {
c := &cassVersion{}
if v == "" {
return c, nil
}
return c.unmarshalCQL(nil, []byte(v))
}
func (c *cassVersion) unmarshalCQL(info TypeInfo, data []byte) error {
return c.unmarshal(data)
}
// can be kept as is
func (c *cassVersion) unmarshal(data []byte) error
// these two should just be global, private and have `CassVersion`
parameters instead
func before(v1 CassVersion, v2 CassVersion) bool
func atLeast(v1 CassVersion, v2 CassVersion) bool
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
