Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 42 additions & 12 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,18 @@ var (
ErrClientClosed = errors.New("Client is closed")
)

type Client struct {
type Config struct {
BaseEndpoint string
Logger *log.Logger
Client *http.Client

MaxBatchBytes int
MaxBatchCount int
MaxBatchInterval time.Duration
}

type Client struct {
Config
writeKey string
wg sync.WaitGroup
semaphore semaphore.Semaphore
Expand All @@ -48,17 +51,45 @@ type Client struct {
}

func New(writeKey string) *Client {
return NewWithConfig(writeKey, Config{})
}

func NewWithConfig(writeKey string, config Config) *Client {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think if Config contains the writeKey?

func New(writeKey string) *Client {
  return NewWithConfig(Config{WriteKey: writeKey})
}

func NewWithConfig(config Config) *Client {
...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Configs tend to imply optionality. The writekey is not optional. I am not sure it belongs in the config

conf := getFinalConfig(config)
return &Client{
BaseEndpoint: DefaultBaseEndpoint,
Logger: log.New(os.Stderr, "segment ", log.LstdFlags),
writeKey: writeKey,
Client: http.DefaultClient,
cmap: newConcurrentMap(),
MaxBatchBytes: 500 << 10,
MaxBatchCount: 100,
MaxBatchInterval: 10 * time.Second,
semaphore: make(semaphore.Semaphore, 10),
Config: conf,
writeKey: writeKey,
cmap: newConcurrentMap(),
semaphore: make(semaphore.Semaphore, 10),
}
}

func getFinalConfig(c Config) Config {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getFinalConfig doesn't really explain much.
Could you consider rename/refactor to

func applyDefaultConfig(c *Config){
// sets default inline
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming is hard. How do parents do this

if c.BaseEndpoint == "" {
c.BaseEndpoint = DefaultBaseEndpoint
}

if c.Logger == nil {
c.Logger = log.New(os.Stderr, "segment ", log.LstdFlags)
}

if c.Client == nil {
c.Client = http.DefaultClient
}

if c.MaxBatchBytes <= 0 {
c.MaxBatchBytes = 500 << 10
}

if c.MaxBatchCount <= 0 {
c.MaxBatchCount = 100
}

if c.MaxBatchInterval <= 0 {
c.MaxBatchInterval = 10 * time.Second
}

return c
}

func (c *Client) fetchFunction(key string) *buffer {
Expand Down Expand Up @@ -167,7 +198,6 @@ func (c *Client) makeRequest(request *batch) {
return
}


b := backoff.NewExponentialBackOff()
b.MaxElapsedTime = 10 * time.Second
err = backoff.Retry(func() error {
Expand All @@ -184,7 +214,7 @@ func (c *Client) makeRequest(request *batch) {

if resp.StatusCode != http.StatusOK {
return fmt.Errorf("HTTP Post Request Failed, Status Code %d. \nResponse: %v \nRequest payload: %v",
resp.StatusCode, response, string(payload))
resp.StatusCode, response, string(payload))
}

return nil
Expand Down