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

Change default config file and verbosity of log #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

stuxboulot
Copy link

fix #1 and #2

Change configuration default file
Chang log format from Fatal to Info

Fix ContentSquare#1 and ContentSquare#2
build default config path from user.path
Change message logs verbosity
change format of file
Copy link
Contributor

@vfoucault vfoucault left a comment

Choose a reason for hiding this comment

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

I'm not sure this is working as expected.
Could you double check your PR

if err != nil {
log.Fatalf("Unable to parse response. response is %v. err=%v", string(body), err.Error())
}
if response.Id == 0 {
// no id in response
log.Fatalf("error sending annotation. Message is %v", response.Message)
log.Info("Sending annotation. Message is ", response.Message)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we fail whenever there is no id, meaning no message ?

Copy link
Author

@stuxboulot stuxboulot Apr 18, 2019

Choose a reason for hiding this comment

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

With my grafana (4.6.3) version i always have Id =0

[14:34:53] p096083@gcf-dev-appv3:~ $ ./grafana-annotation -config-file ~/.grafana-anotation-poster.yml -tag testjio
grafana-annotation 2019/04/18 14:35:11 [INFO] ▶ response Status:200 OK
grafana-annotation 2019/04/18 14:35:11 [INFO] ▶ response Headers:map[Content-Type:[application/json] Date:[Thu, 18 Apr 2019 12:34:59 GMT] Content-Length:[39]]
grafana-annotation 2019/04/18 14:35:11 [CRIT] ▶ error sending annotation. Message is Graphite annotation added
[14:35:11] p096083@gcf-dev-appv3:~ $ ./grafana-annotation -config-file ~/.grafana-anotation-poster.yml -tag testjio
grafana-annotation 2019/04/18 14:37:07 [INFO] ▶ response Status:200 OK
grafana-annotation 2019/04/18 14:37:07 [INFO] ▶ response Headers:map[Content-Length:[39] Content-Type:[application/json] Date:[Thu, 18 Apr 2019 12:36:55 GMT]]
grafana-annotation 2019/04/18 14:37:07 [CRIT] ▶ error sending annotation. Message is Graphite annotation added
[14:37:07] p096083@gcf-dev-appv3:~ $ ./grafana-annotation -config-file ~/.grafana-anotation-poster.yml -tag testjio
grafana-annotation 2019/04/18 14:37:11 [INFO] ▶ response Status:200 OK
grafana-annotation 2019/04/18 14:37:11 [INFO] ▶ response Headers:map[Content-Type:[application/json] Date:[Thu, 18 Apr 2019 12:36:58 GMT] Content-Length:[39]]
grafana-annotation 2019/04/18 14:37:11 [CRIT] ▶ error sending annotation. Message is Graphite annotation added

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Hum, I must say that grafana 5+ is required for this to work properly. I'll add this requirement in the Readme

} else {
log.Info("response Body:", string(body))
log.Fatalf("response Body:", string(body))
Copy link
Contributor

Choose a reason for hiding this comment

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

No way this is correct, response.Id != 0, meaning that we have a result for it. We wont fatalf here.

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.

default config file doesn't work
2 participants