LGTM. Ship it

On Thu, Oct 10, 2013 at 11:57 AM, Ian Booth <email address hidden> wrote:
> Reviewers: mp+190282_code.launchpad.net,
>
> Message:
> Please take a look.
>
> Description:
> Agents log using debug by default
>
> This branch changes the default logging level for agents to use debug.
> The level is set to debug only if the user has not specified something
> different via logging-config or JUJU_LOGGING_CONFIG
>
> https://code.launchpad.net/~wallyworld/juju-core/default-debug-logging/+merge/190282
>
> (do not edit description out of merge proposal)
>
>
> Please review this at https://codereview.appspot.com/14592044/
>
> Affected files (+31, -1 lines):
>    A [revision details]
>    M environs/config/config.go
>    M environs/config/config_test.go
>
>
> Index: [revision details]
> === added file '[revision details]'
> --- [revision details]  2012-01-01 00:00:00 +0000
> +++ [revision details]  2012-01-01 00:00:00 +0000
> @@ -0,0 +1,2 @@
> +Old revision: tarmac-20131009221722-1lb0kalirzqc6s0b
> +New revision: <email address hidden>
>
> Index: environs/config/config.go
> === modified file 'environs/config/config.go'
> --- environs/config/config.go   2013-10-04 12:18:17 +0000
> +++ environs/config/config.go   2013-10-10 00:49:51 +0000
> @@ -104,7 +104,15 @@
>                 if environmentValue := os.Getenv(osenv.JujuLoggingConfig);
> environmentValue != "" {
>                         c.m["logging-config"] = environmentValue
>                 } else {
> -                       c.m["logging-config"] = loggo.LoggerInfo()
> +                       //TODO(wallyworld) - 2013-10-10 bug=1237731
> +                       // We need better way to ensure default logging is set to debug.
> +                       // This is a *quick* fix to get 1.16 out the door.
> +                       loggoConfig := loggo.LoggerInfo()
> +                       if loggoConfig != "<root>=WARNING" {
> +                               c.m["logging-config"] = loggoConfig
> +                       } else {
> +                               c.m["logging-config"] = "<root>=DEBUG"
> +                       }
>                 }
>         }
>
>
>
> Index: environs/config/config_test.go
> === modified file 'environs/config/config_test.go'
> --- environs/config/config_test.go      2013-09-27 02:00:34 +0000
> +++ environs/config/config_test.go      2013-10-10 00:49:51 +0000
> @@ -32,6 +32,11 @@
>
>   var _ = gc.Suite(&ConfigSuite{})
>
> +func (s *ConfigSuite) SetUpTest(c *gc.C) {
> +       s.LoggingSuite.SetUpTest(c)
> +       loggo.ResetLoggers()
> +}
> +
>   // sampleConfig holds a configuration with all required
>   // attributes set.
>   var sampleConfig = testing.Attrs{
> @@ -90,6 +95,14 @@
>                         "default-series": "",
>                 },
>         }, {
> +               about:       "Explicit logging",
> +               useDefaults: config.UseDefaults,
> +               attrs: testing.Attrs{
> +                       "type":           "my-type",
> +                       "name":           "my-name",
> +                       "logging-config": "juju=INFO",
> +               },
> +       }, {
>                 about:       "Explicit authorized-keys",
>                 useDefaults: config.UseDefaults,
>                 attrs: testing.Attrs{
> @@ -721,6 +734,12 @@
>                 c.Assert(cfg.SSLHostnameVerification(), gc.Equals, v)
>         }
>
> +       if v, ok := test.attrs["logging-config"]; ok {
> +               c.Assert(cfg.LoggingConfig(), gc.Equals, v)
> +       } else {
> +               c.Assert(cfg.LoggingConfig(), gc.Equals, "<root>=DEBUG")
> +       }
> +
>         url, urlPresent := cfg.ImageMetadataURL()
>         if v, _ := test.attrs["image-metadata-url"].(string); v != "" {
>                 c.Assert(url, gc.Equals, v)
> @@ -764,6 +783,7 @@
>         attrs["ca-private-key"] = ""
>         attrs["image-metadata-url"] = ""
>         attrs["tools-url"] = ""
> +       attrs["logging-config"] = "<root>=DEBUG"
>         // Default firewall mode is instance
>         attrs["firewall-mode"] = string(config.FwInstance)
>         c.Assert(cfg.AllAttrs(), gc.DeepEquals, attrs)
>
>
>
>
>
> --
> https://code.launchpad.net/~wallyworld/juju-core/default-debug-logging/+merge/190282
> You are subscribed to branch lp:juju-core.