mathiasschw-db commented on code in PR #30: URL: https://github.com/apache/spark-connect-go/pull/30#discussion_r1666519280
########## spark/client/channel/channel.go: ########## @@ -37,27 +39,61 @@ import ( // Reserved header parameters that must not be injected as variables. var reservedParams = []string{"user_id", "token", "use_ssl"} -// Builder is used to parse the different parameters of the connection +// Builder is the interface that is used to implement different patterns that +// create the GRPC connection. +// +// This allows other consumers to plugin custom authentication and authorization +// handlers without having to extend directly the Spark Connect code. +type Builder interface { + Build(ctx context.Context) (*grpc.ClientConn, error) + Headers() map[string]string + Host() string + Port() int + Token() string + User() string +} + +// BaseBuilder is used to parse the different parameters of the connection // string according to the specification documented here: // // https://github.com/apache/spark/blob/master/connector/connect/docs/client-connection-string.md -type Builder struct { - Host string - Port int - Token string - User string - Headers map[string]string +type BaseBuilder struct { + host string + port int + token string + user string + headers map[string]string +} + +func (cb BaseBuilder) Host() string { Review Comment: You should always use pointer receivers for structs. It becomes messy when some functions are on the pointer type and some are not -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org