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

Fixed logging prefixes after moving state/api -> api/ and state/apiserver -> apiserver/ #659

Merged
merged 1 commit into from Sep 2, 2014

Conversation

dimitern
Copy link

@dimitern dimitern commented Sep 2, 2014

After #655 got merged, I noticed some loggers are still using "state.api." and "state.apiserver.", so this fixes these to drop the "state." prefix.

Also, added more logging to juju/testing/conn.go and state.Close() hoping to provide more context for fixing CI bugs like http://pad.lv/1348477.

@mattyw
Copy link
Contributor

mattyw commented Sep 2, 2014

LGTM. I'd recomend posting something to the juju mailing list to make people aware that the logger names are changing - just in case anyone is relying on them.

You can land it. But I'd like @jameinel to review my review

@dimitern
Copy link
Author

dimitern commented Sep 2, 2014

Thanks!
Good point about sending a notice to the ML, doing that now.

@jameinel
Copy link
Member

jameinel commented Sep 2, 2014

As matty mentioned, I'm a little concerned about changing the paths in the log, since it affects people doing custom logging config, but probably the new names are enough better it is worth it.

LGTM

@dimitern
Copy link
Author

dimitern commented Sep 2, 2014

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Sep 2, 2014

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

jujubot added a commit that referenced this pull request Sep 2, 2014
Fixed logging prefixes after moving state/api -> api/ and state/apiserver -> apiserver/

After #655 got merged, I noticed some loggers are still using "state.api.*" and "state.apiserver.*", so this fixes these to drop the "state." prefix.

Also, added more logging to juju/testing/conn.go and state.Close() hoping to provide more context for fixing CI bugs like http://pad.lv/1348477.
@jujubot jujubot merged commit 1060f2d into juju:master Sep 2, 2014
@dimitern dimitern deleted the fix-logging-after-api-move branch September 3, 2014 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants