Skip to content
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

feat:error returning connection in provider method #212

Closed
wants to merge 4 commits into from
Closed

feat:error returning connection in provider method #212

wants to merge 4 commits into from

Conversation

lyonnee
Copy link

@lyonnee lyonnee commented Apr 10, 2023

#210
As @jxsl13 says here, it is necessary to return error in the Provider method. this allows the caller to know the reason for the connection failure

@knadh
Copy link
Owner

knadh commented Apr 10, 2023

Can you also update the instances in https://github.com/knadh/koanf/blob/master/examples/complex-etcd/main.go?

@lyonnee
Copy link
Author

lyonnee commented Apr 10, 2023

Can you also update the instances in https://github.com/knadh/koanf/blob/master/examples/complex-etcd/main.go?

This instance has been modified

feat:error returning connection in provider method

feat: modify etcd provider example
@knadh
Copy link
Owner

knadh commented Apr 12, 2023

clientv3.Config{
   Endpoints:   cfg.Endpoints,
   DialTimeout: cfg.DialTimeout,
}

I think it's best if we pass this etcd config also to the provider config. Otherwise, we're rendering many etcd config options inaccessible.

type Config struct {
        ClientConfig clientv3.Config

	// prefix request option
	Prefix bool

	// limit request option
	Limit bool

	// number of limited pairs
	NLimit int64

	// key, key with prefix, etc.
	Key string
}

@lyonnee
Copy link
Author

lyonnee commented Apr 12, 2023

type Etcd struct {
	client *clientv3.Client
	cfg    Config
}

The configuration information is already stored in this structure

// /providers/etcd/etcd.go
func (e *Etcd) getConf(){
	eps := e.cfg.Endpoints
	dt := e.cfg.DialTimeout

	fmt.Println(eps)
	fmt.Println(dt)
}

@knadh
Copy link
Owner

knadh commented Apr 13, 2023

etcd/clientv3.Config has a lot more options than just Endpoints and DialTimeout. The provider config can accept the whole clientv3.Config as is.

@rhnvrm
Copy link
Collaborator

rhnvrm commented Apr 16, 2023

Maybe, can also consider embedding clientv3.Config and still allow the extra configs like prefix, limit, nlimit, key needed by the koanf provider?

diff --git a/providers/etcd/etcd.go b/providers/etcd/etcd.go
index c75da16..db6f3b9 100644
--- a/providers/etcd/etcd.go
+++ b/providers/etcd/etcd.go
@@ -3,18 +3,11 @@ package etcd
 import (
 	"context"
 	"errors"
-	"time"
 
 	clientv3 "go.etcd.io/etcd/client/v3"
 )
 
 type Config struct {
-	// etcd endpoints
-	Endpoints []string
-
-	// timeout
-	DialTimeout time.Duration
-
 	// prefix request option
 	Prefix bool
 
@@ -26,6 +19,8 @@ type Config struct {
 
 	// key, key with prefix, etc.
 	Key string
+
+	clientv3.Config
 }
 
 // Etcd implements the etcd config provider.
@@ -36,12 +31,7 @@ type Etcd struct {
 
 // Provider returns a provider that takes etcd config.
 func Provider(cfg Config) *Etcd {
-	eCfg := clientv3.Config{
-		Endpoints:   cfg.Endpoints,
-		DialTimeout: cfg.DialTimeout,
-	}
-
-	c, err := clientv3.New(eCfg)
+	c, err := clientv3.New(cfg.Config)
 	if err != nil {
 		return nil
 	}

@lyonnee
Copy link
Author

lyonnee commented Apr 25, 2023

etcd/clientv3.Config has a lot more options than just Endpoints and DialTimeout. The provider config can accept the whole clientv3.Config as is.

Sorry, I didn't understand your meaning before. I have made modifications

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants