Skip to content

Add Apache Aurora Input Plugin#4

Merged
connorgorman merged 1 commit intomasterfrom
aurora
May 11, 2017
Merged

Add Apache Aurora Input Plugin#4
connorgorman merged 1 commit intomasterfrom
aurora

Conversation

@connorgorman
Copy link
Copy Markdown

Required for all PRs:

  • CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

@connorgorman connorgorman requested a review from NDonatucci May 9, 2017 17:50
func (a *Aurora) SetDefaults() {
if a.Timeout == 0 {
log.Println("I! [aurora] Missing timeout value, setting default value (1000ms)")
a.Timeout = 1000
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't this be the value found in SampleConfig?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Or maybe the value in sampleConfig should be that one

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed to be 1000 everywhere

return val, true
}
if val, err = strconv.ParseBool(value); err != nil {
return val.(bool), false
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can this assertion cause an error?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Turns out I don't need the cast. Thanks

a.SetDefaults()

client := http.Client{
Timeout: time.Duration(a.Timeout) * time.Second,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How long is this supposed to be?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch. That's too long

@connorgorman connorgorman merged commit 6510de9 into master May 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants