juju-core

Merge lp:~rogpeppe/juju-core/290-state-less-logging into lp:~juju/juju-core/trunk

  1. 290-state-less-logging
  2. Merge into trunk
Proposed by Roger Peppe
Status: Merged
Approved by: William Reade
Approved revision: no longer in the source branch.
Merged at revision: 1179
Proposed branch: lp:~rogpeppe/juju-core/290-state-less-logging
Merge into: lp:~juju/juju-core/trunk
Diff against target: 88 lines (+11/-6) (has conflicts)
4 files modified
environs/maas/environ.go (+9/-0)
state/megawatcher.go (+0/-4)
state/unit.go (+1/-1)
worker/provisioner/provisioner_test.go (+1/-1)
Text conflict in environs/maas/environ.go
To merge this branch: bzr merge lp:~rogpeppe/juju-core/290-state-less-logging
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+159315@code.launchpad.net

Description of the change

state: lose misleading log messages

https://codereview.appspot.com/8821043/

To post a comment you must log in.
Revision history for this message
Roger Peppe (rogpeppe) wrote :
Download full text (3.8 KiB)

Reviewers: mp+159315_code.launchpad.net,

Message:
Please take a look.

Description:
state: lose misleading log messages

https://code.launchpad.net/~rogpeppe/juju-core/290-state-less-logging/+merge/159315

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8821043/

Affected files:
   A [revision details]
   M environs/maas/environ.go
   M state/megawatcher.go
   M state/unit.go
   M worker/provisioner/provisioner_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: <email address hidden>
+New revision: <email address hidden>

Index: state/megawatcher.go
=== modified file 'state/megawatcher.go'
--- state/megawatcher.go 2013-04-16 14:57:08 +0000
+++ state/megawatcher.go 2013-04-17 08:22:26 +0000
@@ -3,7 +3,6 @@
  import (
   "fmt"
   "labix.org/v2/mgo"
- "launchpad.net/juju-core/log"
   "launchpad.net/juju-core/state/api/params"
   "launchpad.net/juju-core/state/multiwatcher"
   "launchpad.net/juju-core/state/watcher"
@@ -224,7 +223,6 @@
  func (s *backingStatus) updated(st *State, store *multiwatcher.Store, id
interface{}) error {
   parentId, ok := backingEntityIdForGlobalKey(id.(string))
   if !ok {
- log.Errorf("status for entity with unrecognised global key %q", id)
    return nil
   }
   info0 := store.Get(parentId)
@@ -264,7 +262,6 @@
  func (s *backingConstraints) updated(st *State, store *multiwatcher.Store,
id interface{}) error {
   parentId, ok := backingEntityIdForGlobalKey(id.(string))
   if !ok {
- log.Errorf("constraints for entity with unrecognised global key %q", id)
    return nil
   }
   info0 := store.Get(parentId)
@@ -299,7 +296,6 @@
  func (s *backingSettings) updated(st *State, store *multiwatcher.Store, id
interface{}) error {
   parentId, url, ok := backingEntityIdForSettingsKey(id.(string))
   if !ok {
- log.Errorf("settings for entity with unrecognized key %q", id)
    return nil
   }
   info0 := store.Get(parentId)

Index: state/unit.go
=== modified file 'state/unit.go'
--- state/unit.go 2013-04-14 22:12:49 +0000
+++ state/unit.go 2013-04-17 08:22:26 +0000
@@ -668,7 +668,7 @@
   pudoc := unitDoc{}
   err = u.st.units.Find(D{{"_id", u.doc.Principal}}).One(&pudoc)
   if err == mgo.ErrNotFound {
- return "", NotFoundf("principal unit %q", u, u.doc.Principal)
+ return "", NotFoundf("principal unit %q of %q", u.doc.Principal, u)
   } else if err != nil {
    return "", err
   }

Index: environs/maas/environ.go
=== modified file 'environs/maas/environ.go'
--- environs/maas/environ.go 2013-04-14 22:12:49 +0000
+++ environs/maas/environ.go 2013-04-17 08:22:26 +0000
@@ -325,7 +325,7 @@
  // implementation of StartInstance, and to initialize the bootstrap node.
  func (environ *maasEnviron) obtainNode(machineId string, stateInfo
*state.Info, apiInfo *api.Info, tools *state.Tools, mcfg
*cloudinit.MachineConfig) (*maasInstance, error) {

- log.Debugf("environs/maas: starting machine %s in $q running tools
version %q from %q", machineId, env...

Read more...

Revision history for this message
William Reade (fwereade) wrote :

LGTM trivial

https://codereview.appspot.com/8821043/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

LGTM

https://codereview.appspot.com/8821043/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

*** Submitted:

state: lose misleading log messages

R=fwereade, dimitern
CC=
https://codereview.appspot.com/8821043

https://codereview.appspot.com/8821043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'environs/maas/environ.go'
2--- environs/maas/environ.go 2013-04-16 07:37:30 +0000
3+++ environs/maas/environ.go 2013-04-17 09:35:33 +0000
4@@ -286,6 +286,7 @@
5
6 // obtainNode allocates and starts a MAAS node. It is used both for the
7 // implementation of StartInstance, and to initialize the bootstrap node.
8+<<<<<<< TREE
9 func (environ *maasEnviron) obtainNode(machineId string, cons constraints.Value, possibleTools tools.List, mcfg *cloudinit.MachineConfig) (_ *maasInstance, err error) {
10 series := possibleTools.Series()
11 if len(series) != 1 {
12@@ -293,6 +294,14 @@
13 }
14 var instance *maasInstance
15 if node, tools, err := environ.acquireNode(cons, possibleTools); err != nil {
16+=======
17+func (environ *maasEnviron) obtainNode(machineId string, stateInfo *state.Info, apiInfo *api.Info, tools *state.Tools, mcfg *cloudinit.MachineConfig) (*maasInstance, error) {
18+
19+ log.Debugf("environs/maas: starting machine %s in %q running tools version %q from %q", machineId, environ.name, tools.Binary, tools.URL)
20+
21+ node, err := environ.acquireNode()
22+ if err != nil {
23+>>>>>>> MERGE-SOURCE
24 return nil, fmt.Errorf("cannot run instances: %v", err)
25 } else {
26 instance = &maasInstance{&node, environ}
27
28=== modified file 'state/megawatcher.go'
29--- state/megawatcher.go 2013-04-16 16:31:53 +0000
30+++ state/megawatcher.go 2013-04-17 09:35:33 +0000
31@@ -3,7 +3,6 @@
32 import (
33 "fmt"
34 "labix.org/v2/mgo"
35- "launchpad.net/juju-core/log"
36 "launchpad.net/juju-core/state/api/params"
37 "launchpad.net/juju-core/state/multiwatcher"
38 "launchpad.net/juju-core/state/watcher"
39@@ -225,7 +224,6 @@
40 func (s *backingStatus) updated(st *State, store *multiwatcher.Store, id interface{}) error {
41 parentId, ok := backingEntityIdForGlobalKey(id.(string))
42 if !ok {
43- log.Errorf("status for entity with unrecognised global key %q", id)
44 return nil
45 }
46 info0 := store.Get(parentId)
47@@ -265,7 +263,6 @@
48 func (s *backingConstraints) updated(st *State, store *multiwatcher.Store, id interface{}) error {
49 parentId, ok := backingEntityIdForGlobalKey(id.(string))
50 if !ok {
51- log.Errorf("constraints for entity with unrecognised global key %q", id)
52 return nil
53 }
54 info0 := store.Get(parentId)
55@@ -300,7 +297,6 @@
56 func (s *backingSettings) updated(st *State, store *multiwatcher.Store, id interface{}) error {
57 parentId, url, ok := backingEntityIdForSettingsKey(id.(string))
58 if !ok {
59- log.Errorf("settings for entity with unrecognized key %q", id)
60 return nil
61 }
62 info0 := store.Get(parentId)
63
64=== modified file 'state/unit.go'
65--- state/unit.go 2013-04-14 22:12:49 +0000
66+++ state/unit.go 2013-04-17 09:35:33 +0000
67@@ -668,7 +668,7 @@
68 pudoc := unitDoc{}
69 err = u.st.units.Find(D{{"_id", u.doc.Principal}}).One(&pudoc)
70 if err == mgo.ErrNotFound {
71- return "", NotFoundf("principal unit %q", u, u.doc.Principal)
72+ return "", NotFoundf("principal unit %q of %q", u.doc.Principal, u)
73 } else if err != nil {
74 return "", err
75 }
76
77=== modified file 'worker/provisioner/provisioner_test.go'
78--- worker/provisioner/provisioner_test.go 2013-04-16 13:39:29 +0000
79+++ worker/provisioner/provisioner_test.go 2013-04-17 09:35:33 +0000
80@@ -178,7 +178,7 @@
81 resync = time.After(50 * time.Millisecond)
82 s.State.StartSync()
83 case <-timeout:
84- c.Fatalf("machine %v wait timed out")
85+ c.Fatalf("machine %v wait timed out", m)
86 }
87 }
88 }

Subscribers

People subscribed via source and target branches