-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add solr client. #106
Add solr client. #106
Conversation
solr/kubedb_client_builder.go
Outdated
} | ||
|
||
switch { | ||
case version.Major() == 9: |
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.
should be - version.Major() >= 9:
for future compatability
solr/kubedb_client_builder.go
Outdated
config := Config{ | ||
host: o.url, | ||
transport: &http.Transport{ | ||
IdleConnTimeout: time.Minute * 6, |
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.
Why do we have 6 minutes (such long periods) of timeouts everywhere? and can we use a variable for it?
) | ||
|
||
type Client struct { | ||
SLClient |
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.
Can we make this pointer type?
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.
can't use pointer for embedded types
solr/kubedb_client_builder.go
Outdated
return &Client{ | ||
&SLClientV8{ | ||
Client: newClient, | ||
log: config.log, |
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.
If Config have log inside it, do we need to set log as a separate parameter here? Can't we extract log from config later.
return nil, gerr.Wrap(err, "failed to parse version") | ||
} | ||
|
||
switch { |
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.
Do we really need different switch cases for v8 and v9? are there differences in their clients/method implementations ?
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.
some v2 apis are not supported by version 8
solr/solrv8.go
Outdated
} | ||
|
||
func (sc *SLClientV8) ReadCollection() (*Response, error) { | ||
sc.Config.log.V(5).Info("READING COLLECTION: kubedb-system") |
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.
kubedb-system
should be used as a constant everywhere
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
Signed-off-by: pritamdas99 <[email protected]>
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.
LGTM!
Signed-off-by: pritamdas99 [email protected]