[00:10] davecheney: m a bit concerned with the work that you are doing in returning *all* errors that hav previously been logged [00:10] davecheney: what is the bug that set u on this warpath? [00:12] davecheney: for example, m not convinced that callback failure that meant to set the status should really fail the whole operation of container creation [00:13] davecheney: m sure that there are other instances where encounting error should not affect the whole operation. m specifically thinking of simplestreams but thre are others... [00:21] if err != nil { [00:21] logger.Warningf("FIXME: ignoring unexpected CIDR format %q: %v", netConfig.CIDR, err) [00:21] anastasiamac_: here is my philosophy [00:22] if an error occurs one of two things happen [00:22] a. the error means you cannot continue processing because you are missing the requireed data [00:22] b. the error does not stop processing [00:22] if a, then we don't want to log the error asynchrnously into a log file, we want to return it to the caller synchronusly [00:23] if b, then it isn't an error because we can continue [00:23] anastasiamac_: can you point out a specific part of the change you are talking about [00:28] davecheney: for example, container/lxc/lxc.go, line 257... this now means that if u could not successfully callback to set the status, you'd fail container creation [00:31] davecheney: m looking at what u've committed yesterday... and m a bit puzzled... some of the areas did not seem like they warrented to throw an error... and I recall considerable amount of logic in some areas where it was ok to get an error and continue (ur b) [00:31] so we'd log error at another level (like warning or info) and keep going..because it's not significant enought to do anything about but might be of interest during debug [00:33] anastasiamac_: this line ? https://github.com/juju/juju/commit/19f0dbb069916c83007b3f7293e079cb5e450fa4#diff-edf2b984c531037601cf518c0a0b039eL256 [00:33] if we can keep going then it isn't an error [00:36] davecheney: yes, this line. that's the point.. it's not a show-stopper. it's might be useful for when you are diagnosing a field failure - what has happenned leading to my failure - but it's not signifiant enough to stop the operation... [00:36] we have millions of places like this [00:38] yes, and they all need to be fixed [00:38] here is my rule of thumb [00:38] no warnings, ever [00:38] a call to some function may fail but we either have other means of achieving what we want or we don't really care enough [00:38] it's either something the user needs to know about because they need to fix it, or we don't say anything [00:38] so, if we don't care if it succeeds or fails, [00:38] 1. why do we call it [00:39] 2. why do we log the result [00:40] logs are not jsut for user.. they are also for diagnosing field failures... warning means "this fails, we may need to worry about it if something else goes worng or at a later stage or " [00:41] no, i'm sorry, my policy on warnings is absolute [00:41] no warnings, ever [00:41] it's either an error or an info message [00:41] i do think that this should not be tackled at a global level and cetainly not without further discussion [00:41] i don't think the ideas of not having warnings is contraversial [00:41] i'll send a PR to fix lxc/lxc.go [00:42] sure... m not talking about log levels really \o/ my concern is that you are returning errors where they should not be propagated [00:42] plz consider all other places where u have started throwing erros too \o/ u'd know best what was touched :D [00:43] what are you asking ? [00:43] the code was reviewed, all the tests pass, juju.fail is clear [00:43] i there are any other places similar to https://github.com/juju/juju/commit/19f0dbb069916c83007b3f7293e079cb5e450fa4#diff-edf2b984c531037601cf518c0a0b039eL256 [00:44] this sounds like fear that something that we don't know about might break because we don't know about it because its scarey adn we don't trust out code review or our tests [00:44] plz revert them whenevr possible.. (i dunno if there are - i trust u to know) \o/ [00:44] as for log levels, there is a team meeting tonight at our 8pm (well, bne at least) [00:44] let's discuss it then \o/ [00:45] davecheney: i agree that there mayb some places where we are swallowing errors..but i'd like these to be fixed on a per-need basis, prefereably with a bug backing the need [00:46] anastasiamac_: https://github.com/juju/juju/pull/5432 [00:58] davecheney: FWIW, all the places where we appear to swallow an error need to have a clear explanation as to why... [00:59] i agree [00:59] davecheney: i have seen instances where these are explained in the review comments... but i believe explanantion needs to be in the codebase [00:59] davecheney: \o/ [00:59] but stil, don't log errors at warning level [00:59] davecheney: plz come to team meeting - let's talk :D [01:00] anastasiamac_: all the decision makers will be asleep [01:00] they are all in the US timezone [01:01] davecheney: i don't think it requires executive decision :D just ocnsensus and europe will b online [01:01] why do we need to have a call [01:01] bleh.. consensus :D [01:01] can't we just talk about it now [01:01] or send an email ? [01:02] email is just as good but f-2f may be more efficient [01:03] personal preference, for majority of cases - errors should b logged at error-level [01:03] i just can;t think of a places where it would not need to.. and don;t want to omit someplaces that other ppl mayb aware of :D [01:05] have a look at the branches i've been landing [01:05] most of this is s/Warningf/Errorf [01:06] with the exceptoins where we do something like [01:06] Warningf("ignore error, carrying on anyway") [01:06] in whcih case that is debugging information because there is nothing a user can do with this knowledge [01:06] that's just for us developers [01:14] well, I think that as long as there is a good explanation in the comment and in the log message as to why an error is ignored, whether it it's logged t Warning or Error level is less important... Explanation is what really matters :D [01:14] FWIW... I hate errors in logs [01:15] unless it's a showstopper [01:15] natefinch: warning might be better as we aer not treating it as an error ;D [01:15] agreed \o/ all errors proper should be logged at error-level [01:15] nope, I'm with Dave on that one. Warnings are dumb. Debug, Info, Error, and error should almost never be used [01:17] the problem is that, to anyone who is not the specific developer who wrote that warning/error log... it's impossible to know if an ERROR is really an ERROR, or just "hey, something went wrong, that's ok" [01:17] yeah :D i bbelieve that all traffic lights should loose yellow - u other stop or u go :) [01:17] anastasiamac_: there is no need to be facetious [01:17] here is my rule of thub [01:17] yellow doesn't mean "maybe it's ok, maybe it's not" [01:17] but that's what warning is [01:17] tell the user if there is something they need to do [01:17] warning-level is like yeelow :D [01:17] sorry, no, that wasn't right [01:18] m not being f** :D [01:18] tell the user if there is something they can do [01:18] if there is nothing the user can do, then don't tell them [01:18] either carry on, or the program quits [01:19] anastasiamac_: for reference, http://dave.cheney.net/2015/11/05/lets-talk-about-logging [01:19] davecheney: m not convinced that in ideal world user should need to look in the logs (only when something went horribly wrong, the logs should be trotted) [01:19] user should receive an intelegent feedback.. [01:19] the logs are for developers [01:19] or extreme power users [01:20] +100 \o/ [01:20] anastasiamac_: logs are for develoeprs [01:20] the return value of a function is for the user [01:21] davecheney: i hope u mean return value of an operation... our functions should not matter to user eaither [01:21] function/method/opertation, what'ever works for you [01:22] davecheney: granularity is different [01:22] i disagree [01:23] if as auser I want to add a machine, i don't care if u r calling 100 functions underneath (and some of the functions may not make sense to me) [01:23] i agree [01:23] that is what I meant [01:24] an operation either worked or it didn't [01:24] if it didn't it returns an error [01:24] \o/ m glad we agree [01:24] it should not fail, but rturn no error while poopping out some warning messages on the side [01:25] this is what I am working on fixing at the moment [01:27] logging at ERROR level is confusing to *developers* though. Does it mean juju is in a bad state? What *does* it mean? If we can ignore it, it must not really be that much of an error. Warning is just as bad. [01:27] WARNING juju.worker.uniter upgrade126.go:26 no uniter state file found for unit unit-resource-0, skipping uniter upgrade step [01:27] is my juju broken? is it a bug? [01:28] I presume that is actually a perfectly fine state for juju to be in... but I don't know [01:28] i think it's ok to say "warning: blah blah did not happen, will try bleh".. and as a user m ok to see these go by :D [01:29] it's kind of like saying "my 1st preference is this but it did not happe, i'll try my 2nd beest" [01:29] natefinch: exactly [01:29] (without the typos) [01:29] anastasiamac_: so in that case that's just debugging information [01:31] not necessarily dbugging (althou it can be).. it's more of an operation progress :D [01:33] I'm seeing something very odd with juju2.0 while installing charms. A day ago, my charms were installing fine, then today one is failing in the cloud-init section when trying to run jujud-machine-xx. After connecting to the machine and running initctl reload-configuration, I can now start the job. Otherwise, the error is 'Unknown job' Any ideas? [01:35] anastasiamac_: here is my rule of thumb [01:35] when reading a log file, if you every reach for grep -v to filer out "operating in progress" informational messges [01:35] As far as I can see, the logs are identical between the charms which worked(ie. the cloud-init-output.log) and the one that doesn't. It appears upstart doesn't recognize the script. [01:35] they shouldn't be logged in the first place [01:35] every single line reported to the user must be something actionable [01:35] "i'm still working on this" is not actionable [01:37] man, so much of our logging is utter crap. I mind it less when it's at debug level, because, well, it can be useful for debugging. But info should be something at least useful. important changes to state, etc. [01:57] I think warnings are reasonable for broadcasting information such as API deprecation or change when such an API or other such announcements. However, retries (my first pref didn't happen trying second pref) are a functioning codepath and shouldn't be logged to the end user IMO. [01:57] redir: no, sorry, no warnings ever [01:57] Bug #1583274 changed: Openstack base bundle 42 fails to start rabbitmq when deployed with Juju/MAAS 2.0

[01:57] it's either an error or an info [01:57] when such an api is called [01:58] ok, so that's an informational message "attention: this api call is deprecated and will be removed shortly" [01:58] well I guess if you could then just have it be an error when the API changes or is removed. [01:59] you'll get the error for free when the api method is removed :) [02:00] I usually suppress info only warn and worse when I care about logs in production [02:00] redir: precisely [02:00] which is why I loose my shit when errors are logged at warning level [02:00] for context: http://dave.cheney.net/2015/11/05/lets-talk-about-logging [02:00] I agree 100% errors need to be errors [02:01] been there read that [02:01] :) [02:02] API warnings are probably moot in the context of juju since end users aren't using juju as library and dont' need to know about internal deprecations. So, my bad example. === redir is now known as redir_afk [03:04] ping, https://github.com/juju/juju/pull/5433 [03:04] this is a simple update to depenedncies.tsv [03:07] davecheney: ship it [03:07] honestly, I think that if we're just updating a dependency we wrote, we shouldn't need a review [03:08] natefinch: thanks nate [03:11] if err != nil { [03:11] logger.Warningf("FIXME: ignoring unexpected CIDR format %q: %v", netConfig.CIDR, err) [03:11] continue [03:11] } [03:11] ... i don't even know where to begin [03:11] uh, yeah [03:14] OH MY GOD [03:14] we have an apiserver/common package [03:14] and we have an apiserver/common/networkingcommon package [03:14] HOW DID THIS HAPPEN !? [03:14] lol [03:14] common, but not that common [03:14] sorta in the in crew, but not really [03:15] Bug #1583409 opened: help on status-history is confusing [03:15] Bug #1583412 opened: status-history doesn't complain about badly formatted names [03:16] is status-history supposed to work? [03:18] you might have to be flexbile in your definition on work [03:18] well, mine seems to always just return "ERROR no status history available" [03:18] Bug #1583409 changed: help on status-history is confusing [03:18] Bug #1583412 changed: status-history doesn't complain about badly formatted names [03:27] hmm, just updated my client and now I get ERROR json: cannot unmarshal object into Go value of type names.Tag [03:27] Bug #1583409 opened: help on status-history is confusing [03:27] Bug #1583412 opened: status-history doesn't complain about badly formatted names [03:30] anastasiamac_: natefinch http://reviews.vapour.ws/r/4870/ [03:30] natefinch: urk, how are tags leaking out of the apiserver [03:31] Bug #1583409 changed: help on status-history is confusing [03:31] Bug #1583412 changed: status-history doesn't complain about badly formatted names [03:31] davecheney: lemme rebuild the server and make sure I haven't screwed things up somehow [03:32] logger.Debugf("coercion failed attributes: %#v, checker: %#v, %v", attrs, checker, err) [03:32] return nil, err [03:32] log an error, at debug, then return a different error [03:34] Bug #1583409 opened: help on status-history is confusing [03:34] Bug #1583412 opened: status-history doesn't complain about badly formatted names [03:41] davecheney: oops: https://github.com/juju/juju/blob/master/apiserver/params/status.go#L138 [03:46] that'll never work [03:46] good think it's got a unit test [04:07] oh, there's even more broken there [04:07] also json related... the request object uses a *time.Time... then checks for nil after deserialization [04:08] except that it serializes as a zero time and so deserializes as a zero (but non-nil) time [04:22] Bug #1583422 opened: status-history is broken [06:04] Bug #1583443 opened: juju 2.0 doesn't support bundles with 'to: ["service=0"]' placement syntax [06:13] Bug #1583443 changed: juju 2.0 doesn't support bundles with 'to: ["service=0"]' placement syntax [06:22] Bug #1583443 opened: juju 2.0 doesn't support bundles with 'to: ["service=0"]' placement syntax [06:31] Bug #1583443 changed: juju 2.0 doesn't support bundles with 'to: ["service=0"]' placement syntax [08:54] dimitern: ping? [08:55] babbageclunk: pong [08:57] dimitern: So, I think I've found the immediate thing causing these tests to fail (although I don't know what it means or how it happens yet) :). [08:57] babbageclunk: oh, go on? [08:57] In a successful test I get volumeattachments with volumeids 0, 0/1, 0/2 [08:58] but in a failing one they are 0/0, 0/1, 2 [08:59] hmm [09:00] What's the difference between the 0/1 ones and the 0 ones? What does the / indicate? [09:00] babbageclunk: weird.. what's expeced format of those IDs? [09:01] babbageclunk: ISTM "0" is the machine ID, whereas "0/1" is the second volume ID on that machine? but might be wrong.. [09:01] dimitern: well, they look more cryptic because 0 is the machine id. [09:01] ha ha yup [09:01] oops standup [09:02] omw [09:02] me 2 [09:02] oh no - nothing until 11 [09:02] technically we don't have scheduled standup today [09:03] we can still do it with yesterday's slot if you like? [09:03] yeah, sure - would be helpful for me! [09:26] babbageclunk: that comment in names/volume.go explains it: Volumes may be bound to a machine, meaning that the volume cannot exist without that machine. We encode this in the tag to allow [09:27] so "0" and "0/1" are both valid volume tags [09:32] babbageclunk: also looking at TestWatchMachineVolumes ISTM the watcher is only started after adding the service, initial unit, and the following addUnit() [09:33] babbageclunk: are you stopping all watchers before resetting the DB [09:34] babbageclunk: it looks like if the watcher keeps going while the DB reset happens, it will still report the initial events *before* it was supposed to start at line 351 in state/volumes_test.go [09:39] dimitern: No, that's probably it - thanks! [09:58] dimitern: heh, so I have three places marked with XXX and no failing tests [09:58] dimitern: moar tests [09:58] dimitern: to be fair - I don't think those tests were needed before, so it's not really a lack of coverage [09:59] voidspace: moar tests == good :) [09:59] dimitern: well, so long as they're not flaky... [10:00] voidspace: oh yeah [10:02] dimitern: mtg? [10:02] omw [10:04] davecheney: call? [10:58] fwereade: re: logging context, I really liked the distinction between spans and baggage in http://opentracing.io/spec/ [11:24] babbageclunk, so baggage is effectively high-level task context, and spans correspond to the various different components that might happen to be doing stuff in response? [11:24] babbageclunk, if so, yeah -- I'm currently thinking spans mainly, but baggage would be a fantastic addition [11:25] katco, worth considering in the context of audit? ^^ [11:27] fwereade: yeah, that's right. At the stock exchange there was a component that stamped every message flowing through the system with an id that let you follow the flow for that message through all of the logs across lots of different subsystems. [11:27] fwereade: Was absolutely essential for tracking stuff down. [11:34] babbageclunk, I have some concerns about how far we can meaningfully take it in juju, because at a certain point everything is happening because of everything else [11:34] babbageclunk, but the useful 80% would be relatively cheap and easy, I think [11:35] babbageclunk, (that point is once you get down to relation settings changes, basically) [11:38] babbageclunk, (and we don't have any useful insight into what relation changes *actually* cause things to happen -- you could probably do something probabilistic with a suitably overarching view of the system, but that's a bit scope-creepy ;p) [11:38] fwereade: :) [11:41] fwereade: Yeah, that system was a lot less dynamic than juju - adding a machine or process while the system running was a rare, scary, fiddly task, where here it's kind of the point. [11:53] Bug #1583564 opened: failed to download URL for lxc images

[11:56] Bug #1583564 changed: failed to download URL for lxc images

[12:02] Bug #1583564 opened: failed to download URL for lxc images

[13:39] voidspace, babbageclunk: please take a look at http://reviews.vapour.ws/r/4871/, which fixes bug 1574844 [13:39] Bug #1574844: juju2 gives ipv6 address for one lxd, rabbit doesn't appreciate it.

[13:39] (I still need to add a few extra unit tests, but otherwise it won't change much) [13:40] * dimitern steps out for ~30m [13:48] Bug #1583564 changed: failed to download URL for lxc images

[13:54] dimitern: Why have duplicate attributes and methods for IPv6 and IPv4 addresses? Why not pass in a type (or use the address.Type() where approro [13:54] Oopp [13:54] Gah. ...where appropriate? [13:59] babbageclunk: because of the way address selection works, you can't rely on addr.Type == network.IPv6Address when trying to set the preferred IPv6 address (e.g. when only IPv4 addresses are available) [13:59] babbageclunk: I'd appreciate suggestions on the review :) [14:00] babbageclunk: like what's unclear and possibly the need for extra comments, etc. [14:00] now I'm really afk for a while.. [14:00] dimitern: Was doing that, but wanted to make sure I wasn't going off half-cocked. [14:01] babbageclunk: fwereade: thanks, i'll give it a read over [14:02] dimitern: why not just have the preferred address algorithms prefer IPv4? [14:03] dimitern: I don't see a need for all these extra methods [14:59] voidspace: because the whole point of the preferred addresses is that they don't change once set [15:01] voidspace: and if providerAddresses = ["public:fc00:123::/64"] the very first time SetProviderAddresses is called, both the preferred public and private IPv4 and IPv6 will be the same and they cannot change if later SetProviderAddresses is called with e.g. ["public:8.8.8.8", "public:fc00:123::/64", "local-cloud:10.0.1.2", "local-cloud:fe80::/64"] [15:02] that's essentially what the bug is about [15:03] voidspace: that's why preferred IPv4 and preferred IPv6 need to be stored separately [15:04] dimitern: hmmm... [15:04] natefinch: standup time [15:05] dimitern: you still don't need two *setters* as preferred ipv4 and ipv6 can be determined from the type of the input to SetPreferredAddresses [15:05] voidspace: you mean for migrations? [15:06] dimitern: I mean you don't need SetPreferredIPv4Addresses and SetPreferredIPv6Addresses [15:06] voidspace: that's for migtations only [15:06] dimitern: migration of what/ [15:06] what? [15:07] voidspace: SetPreferredIPv4Addresses is a method of description.Machine interface in core/descriptiob [15:07] description [15:07] dimitern: I don't think it needs to be [15:07] you certainly don't need the word "Addresses" in there, at the very least. This isn't java. [15:07] dimitern: you can SetPreferredAddresses twice, once with ipv4 addresses and once with ipv6 [15:08] dimitern: and if there's a type switch internally it will "do the right thing" without needing two methods [15:08] ericsnow: natefinch: redir_afk: my hangouts has frozen [15:08] * katco waits for it to unstick [15:09] voidspace: which SetPreferredIPv4Addresses method are you talking about/ [15:09] ? [15:11] dimitern: machine [15:11] dimitern: and of course the interface in core descriptions [15:11] dimitern: have SetPreferredAddresses sort the addresses it is given by type and set both ipv4 and ipv6 preferred addresses [15:12] dimitern: and if it is called twice, once with ipv4 only and once with ipv6 only, it will still "do the right thing" [15:12] voidspace: I don't mind changing the description.Machine methods - they are less relevant anyway and will be likely called with all 4 preferred set [15:12] voidspace: there's no SetPreferredIPv4Addresses on state.Machine (or anything like that) [15:13] dimitern: ah, it's core/description/machine [15:13] dimitern: I care less about extra methods there I guess [15:13] voidspace: yeah :) [15:13] voidspace: my point exactly :) also I was following the pattern for existing migrations interface methods [15:13] dimitern: ok [15:13] sorry, brb again [15:27] is anyone available to help troubleshoot a 1.25 controller stuck in an erroneous "upgrade", the user apparently didn't request the upgrade, but rebooted the host controller and its stuck doing an upgrade. [15:27] they're over in #juju if anyone has a moment to lend a hand. i'm out of my depth on this one, i'm directing them to aggregate logs and file a proper bug [15:42] lazyPower: I noticed he said this was a case study and would get published.... sigh. bad day for all the managers to be at a sprint [15:42] rick_h_: you around? [15:43] agreed :/ [15:50] i also need a volunteer to tal @ bug 1537585. this is blocking the landscape team from releasing. [15:50] Bug #1537585: machine agent failed to register IP addresses, borks agent <2.0-count>

[15:51] natefinch: you're the only one without feature work atm. are you looking at this other customer-facing issue in #juju? [15:51] perrito666: what are you up to? [15:52] katco: was just trying to figure out if I should help the guy in #juju... I've gotten flack for doing that in the past, so was hesitant this time. [15:55] natefinch: i was thinking the same thing. we have paid support for that reason [15:56] natefinch: i would say pick up this bug if you can. it will unblock landscape and make all of juju better [15:56] katco: before or after I fix status-history? [15:57] natefinch: how long do you think it will be for that fix? [15:57] katco: just an hour or so to tweak the tests to pass [15:57] natefinch: i'd say wrap that up first then [15:57] katco: yeah, my thought as well. Cool. [15:57] katco: then I'll hop on the landscape one [15:58] natefinch: you are a bug fixing machine [16:08] katco: I was in bed until about now, I am trying to wrap up the restore work that ian inherited me [16:08] perrito666: ok, ta. hope you're feeling better :( [16:08] katco: nope, but bed is driving me crazy so I get up and work every now and then [16:09] perrito666: sorry to hear that [16:09] katco: you know how these things are, you just have to wait and eat chicken soup [16:09] perrito666: yeah. i'm not a good sick person. [16:24] Bug #1583683 opened: juju thinks it is upgrading after a reboot [16:30] Bug #1583683 changed: juju thinks it is upgrading after a reboot [16:43] Bug #1583683 opened: juju thinks it is upgrading after a reboot === redir_afk is now known as redir [16:57] kat/nate - is there anyone in particular i should ping with the bug dweaver submit for their issue? (the #juju casestudy user) [16:57] katco natefinch ^ - sorry pinging you directly as you made mention. I want to route this appropriately [16:58] lazyPower: i pinged cheryl, alexis, and rick_h_ [16:58] ok, bugs right up there ^ [16:58] lazyPower: ta [16:59] thanks for the handoff [16:59] lazyPower: just want to be crystal clear: we haven't committed to investigating this yet [17:00] ok, if it gets dropped it gets dropped. Theres very little i can do about it, and i know we're on a tight schedule/deadline. [17:00] we've done what we can [17:01] lazyPower: curious if they can hang on until next week? half our resources are sprinting atm [17:02] i made that clear that we were short handed due to a planning sprint. Their response: [11:59:18] dweaver: lazyPower, yes, willing to be patient, thanks. I'll ping you with a bug ID once it is submitted with all the info. Will be most appreciative thanks. [17:02] lazyPower: awesome, ta chuck [17:03] np np, good hustle [17:50] alexisb: are we still doing our 1:1? figured you were busy with sprint [17:50] katco, we are not [17:50] how are things? [17:50] alexisb: things going fine [18:24]