| menn0 | thumper: i get that all the time in my trusty VM | 00:54 |
|---|---|---|
| thumper | ugh | 00:55 |
| menn0 | thumper: we should probably file a bug | 00:55 |
| menn0 | :) | 00:55 |
| anastasiamac | axw: addressed :D PTAL when u get a chance :D | 01:53 |
| axw | anastasiamac: looking | 01:54 |
| anastasiamac | axw: \0/ | 01:54 |
| davecheney | any one having troubles adding comments to reviewboard today | 02:34 |
| davecheney | the comment box does not load for me | 02:34 |
| davecheney | http://reviews.vapour.ws/r/1847/diff/# | 02:35 |
| davecheney | can someone please try to add a comment on this review and let me know if it works or not | 02:35 |
| natefinch | davecheney: worked for me. | 02:40 |
| davecheney | yay, logout / login dance worked | 02:43 |
| davecheney | i hate reviewboard | 02:43 |
| davecheney | it adds negative value | 02:43 |
| natefinch | davecheney: when we started with it, people wanted it for side-by-side diffs and dependent commits. I don't know if anyone really uses the dependent commits (I certainly don't), and Github has side by side diffs now. | 02:47 |
| natefinch | davecheney: it is a lot easier to see a few commits at a time on reviewboard... on github you see everything or just one. And reviewboard makes commenting better, because they're batched up. | 02:48 |
| natefinch | davecheney: it's also way easier to see old comments and the state the code was in at the time. | 02:51 |
| davecheney | sure, no aregument there | 02:54 |
| davecheney | but IMO that doesn't pay for reviewboards inherent shitness as an implementation | 02:55 |
| anastasiamac | davecheney: ty for shipit! | 03:11 |
| anastasiamac | axw: i got t2 for the price of one! tyvm :))) | 03:12 |
| davecheney | thumper: what's going on with this workload stuff | 03:12 |
| anastasiamac | 2 shipits* | 03:12 |
| davecheney | is juju getting into the process management game ? | 03:12 |
| axw | anastasiamac: nps | 03:12 |
| natefinch | davecheney: yes | 03:12 |
| davecheney | is there any value in me advising caution in this respect ? | 03:12 |
| thumper | davecheney: what workload stuff? | 03:13 |
| natefinch | probably not in the grand scheme of things, but I personally would like to hear what you have to say, and since my team is looking into this, that probably is useful. | 03:13 |
| natefinch | current spec (still being revised): https://docs.google.com/document/d/1PcRQXaerlsACro4y1y5LWD-uvhfHya2CkOcoljyFyCU/edit#heading=h.62n9cmrnxg4o | 03:13 |
| natefinch | brb | 03:14 |
| davecheney | natefinch: IMO the only reliable way to track a process is to be it's parent | 03:14 |
| thumper | davecheney: updated my branch | 03:15 |
| davecheney | and as the charms are the ones that own the start hook, it is not possible for juju to be the parent of any process executed by the start hook | 03:16 |
| thumper | davecheney: I hear what you are saying | 03:22 |
| thumper | davecheney: we are going for a good approximation of perfection :) | 03:22 |
| natefinch | davecheney: I think we could have juju be the parent, but it would go against the way everything else works. The main problem is that juju is currently set up such that everything in a charm is a script. Getting juju to be the one to launch the process would likely require a more declarative approach than we currently use. | 03:48 |
| natefinch | davecheney: there is a note in the spec about auto-launching from metadata... so that might do what you want. | 03:50 |
| natefinch | it is under "open questions" and does say "optional" though | 03:50 |
| davecheney | natefinch: i don't think juju can be the parent | 04:11 |
| davecheney | postgress for example WILL NOT run in the foreground | 04:11 |
| thumper | davecheney: could I get you to take another look at http://reviews.vapour.ws/r/1847/ plz? | 04:12 |
| davecheney | and some charms start more than one process, etc | 04:12 |
| davecheney | and when I origianlly joined Juju it was because juju was not going to be a process manager | 04:12 |
| davecheney | thumper: done | 04:18 |
| wallyworld | thumper: does a django charm take a zip / tarball as the app payload? | 04:21 |
| natefinch | way past my bedtime. Dave - we'll do the best we can. I'm more than happy to have your input on it. But I think container management (regardless of what we call it) is sort of required at this point. | 04:22 |
| davecheney | it might be required | 04:26 |
| davecheney | but getting 100% functionality is not possible without being the parent of the process | 04:26 |
| davecheney | and 90% coverage will be like HA, and backups, and leadership, etc | 04:26 |
| davecheney | that is, a source of bugs and suoport calls | 04:26 |
| thumper | wallyworld: no, not at this stage | 04:27 |
| wallyworld | thumper: how does django in general consume its app payload when you run a django server? | 04:27 |
| wallyworld | an exploded dir of files? | 04:28 |
| wallyworld | you give django the dir via some config? | 04:28 |
| thumper | wallyworld: there are two ways right now: 1) use a config value to specify a branch of bzr/git/hg/svn that is your app | 04:29 |
| thumper | which I don't do | 04:29 |
| thumper | or | 04:29 |
| thumper | provide a payload subordinate charm that installs it using the 'django-settings' relation | 04:29 |
| wallyworld | thumper: forgetting charms, if a user installs django without juju | 04:29 |
| wallyworld | how does django itself consume the payload? dir on disk? | 04:30 |
| thumper | yes, an app is normally a python module | 04:30 |
| thumper | or package | 04:30 |
| thumper | always get those two mixed up | 04:30 |
| thumper | dir in the python path | 04:30 |
| thumper | django executable is called with a settings module | 04:31 |
| thumper | it uses those settings to determine which apps are used | 04:31 |
| thumper | apps are expected to be in the python path | 04:31 |
| wallyworld | thumper: thanks, so with resources, we would store an egg of something ad then explode that in the right place for django to pick up | 04:31 |
| thumper | yeah | 04:32 |
| wallyworld | ta | 04:32 |
| thumper | wallyworld: it would certainly be simpler than using a payload charm | 04:32 |
| thumper | as long as the resources are optional | 04:32 |
| wallyworld | yes, we will still support the current methods | 04:33 |
| wallyworld | eg charm specifes bzr url | 04:33 |
| thumper | optional meaning: could be there for any particular instance | 04:33 |
| thumper | not supported or not | 04:33 |
| thumper | it would be good to have an optional resource | 04:33 |
| thumper | so we could use it if specified | 04:33 |
| * thumper wanders off again | 04:34 | |
| wallyworld | yes, but if supported an admin could publish a django app to JES and then juju deploy --resource myapp=webstorev2 | 04:34 |
| davecheney | https://esta.cbp.dhs.gov/ | 04:41 |
| davecheney | anyone seeing a cert error visiting this site ? | 04:41 |
| miken | davecheney: nope (FF 38) | 05:05 |
| davecheney | chrome 43 whinges | 06:38 |
| davecheney | but not chrome on my nexus | 06:38 |
| rogpeppe1 | davecheney: thanks for the review of http://reviews.vapour.ws/r/1853/diff/# | 07:04 |
| rogpeppe1 | davecheney: i've updated accordingly | 07:05 |
| rogpeppe1 | fwereade: fancy taking a look at http://reviews.vapour.ws/r/1853/diff/# ? it starts to implement some of the stuff that we discussed. | 07:23 |
| fwereade | rogpeppe1, lovely, LGTM with a minor | 07:42 |
| rogpeppe1 | fwereade: ta! | 07:42 |
| rogpeppe1 | fwereade: i'd prefer not to add constants in this PR as it will obscure the actual necessary changes. i've left as much code as possible untouched so that it's obvious that it's not that invasive a change. | 07:44 |
| rogpeppe1 | fwereade: ISTM that adding constants is a fix for old code that justifies a separate PR | 07:45 |
| fwereade | rogpeppe1, it doesn't *have* to be this PR, but would you do a followup then? | 07:45 |
| rogpeppe1 | fwereade: sure | 07:45 |
| fwereade | rogpeppe1, works for me | 07:45 |
| fwereade | rogpeppe1, tyvm | 07:45 |
| rogpeppe1 | fwereade: and i agree about the -path keys but that was another bite too big for this stage | 07:46 |
| fwereade | rogpeppe1, definitely | 07:46 |
| rogpeppe1 | fwereade: i wanted to start with a fields var that was identical to the original one (i verified with gc.DeepEquals) | 07:46 |
| fwereade | rogpeppe1, yeah, I do appreciate how uninvasive it is :) | 07:47 |
| rogpeppe1 | fwereade: you'll notice that i added another grouping ("juju") for attributes that are created by juju itself and can't be specified by the user. | 07:48 |
| rogpeppe1 | fwereade: AFAIK agent-version and uuid are the only two members of that group - is that right? | 07:48 |
| fwereade | rogpeppe1, ...I *think* so | 07:52 |
| axw | dimitern: would you mind reviewing a small change to ec2test? https://github.com/go-amz/amz/pull/51 | 08:57 |
| dimitern | axw, sure | 08:57 |
| dimitern | axw, LGTM | 08:58 |
| axw | dimitern: thanks | 08:58 |
| voidspace | dimitern: omw | 09:01 |
| jam | fwereade: TheMue: shouldn't we be archiving cards now that we've looked over the board? | 09:32 |
| TheMue | jam: I don't know the capabilities of the tool. unless there are no limits and good ways to query I always prefer archiving, yes. | 09:34 |
| jam | TheMue: we probably need to work with Alexis so she can pull out whatever metrics she wants to focus on (how many bugs addressed, velocity, etc) | 09:36 |
| TheMue | jam: yep, I'll also talk to katco about the separation between planned tasks in cards and fixes in lp. currently I'm not sure about handling issues in kanban too (double capture vs simple overview in one place) | 09:39 |
| rogpeppe1 | axw: ping | 09:40 |
| axw | rogpeppe1: pong | 09:40 |
| rogpeppe1 | axw: yay! you're there :) | 09:41 |
| rogpeppe1 | axw: i'm looking at UpgradeSuite.SetUpTest in cmd/juju/agent, which i think you might have had a hand in | 09:41 |
| jam | TheMue: lp doesn't make it easy to break down by team IIRC | 09:42 |
| rogpeppe1 | axw: i just saw a test panic, and it looks like it might be related to the go func()... setAptCmds statement | 09:42 |
| jam | and if we do start sizing bugs, it doesn't tarck that either | 09:42 |
| rogpeppe1 | axw: ... possibly :) | 09:42 |
| axw | rogpeppe1: possible dabbled in there. looking | 09:42 |
| rogpeppe1 | axw: anyway, it looks suspicious - do you know why that test starts a goroutine there? | 09:42 |
| TheMue | jam: yes, different focus. would like a kind of plugin to import an issue into a new card in a given board with auto-linking | 09:43 |
| TheMue | *dreaming* | 09:43 |
| axw | rogpeppe1: IIRC is a contortion around the way command execution is hooked | 09:43 |
| axw | rogpeppe1: so the commands are hooked, then sent to a channel, then this goroutine watches that channel and does stuff with them | 09:44 |
| rogpeppe1 | axw: i don't see how it could ever be correct | 09:44 |
| rogpeppe1 | axw: setAptCmds doesn't seem to synchronise with anything | 09:44 |
| natefinch | jam, TheMue: we don't size bugs. we treat them as a kind of overhead | 09:44 |
| rogpeppe1 | axw: i'm fairly convinced this can't have anything to do with the panic i saw (a mgo double-close), but it still looks wrong to me | 09:45 |
| jam | natefinch: depends how you want to do it. That portion is up to the tea | 09:45 |
| jam | team | 09:45 |
| natefinch | jam, TheMue: the reason is that the sizing is so we know how long features take to implement. The bugs factor into the environment that makes features take longer | 09:45 |
| jam | TheMue: you can look into lp2kanban if you like, its a python project on LP that was an attempt to use LP's and Leankits APIs to sync them. | 09:46 |
| axw | rogpeppe1: what do you mean doesn't synchronise? s.aptCmds locks a mutex on the suite...? | 09:46 |
| rogpeppe1 | axw: yes, but there's nothing to stop that goroutine running randomly after the test has finished | 09:46 |
| rogpeppe1 | axw: or even in arbitrary order over several tests | 09:46 |
| axw | rogpeppe1: right. yes, that could be a problem. | 09:46 |
| rogpeppe1 | axw: it's mutexed but not synchronised | 09:46 |
| natefinch | jam, TheMue: if we had bugs in github there are like 1000 projects to sync things :/ for ex: https://waffle.io/juju/juju | 09:47 |
| jam | natefinch: unbound 'done' doesn't seem great from Waffle | 09:47 |
| axw | rogpeppe1: is this happening for you frequently, or sporadic? | 09:48 |
| rogpeppe1 | axw: i just saw it once; haven't tried again | 09:48 |
| axw | ok | 09:48 |
| natefinch | jam: I haven't dived into it super deeply, but I know it's pretty customizable | 09:48 |
| rogpeppe1 | axw: for the record, this is the panic i saw: http://paste.ubuntu.com/11562154/ | 09:48 |
| TheMue | natefinch: I would like bugs on github too | 09:48 |
| axw | rogpeppe1: I'll make a note to fix it, need to finish off some storage stuff | 09:49 |
| rogpeppe1 | axw: ta! | 09:49 |
| rogpeppe1 | axw: it may have something to do with the fact i'm running with go tip, which i believe has changed the default MAXGOPROCS to >1 | 09:50 |
| natefinch | TheMue: from a purely pragmatic viewpoint, having bugs closer to the code is a good thing... plus then you get all the github bug integrations, like "fixes #123" which marks the bug as closed and puts a link from the bug to the fix automatically. I think we'd gain a *lot* by moving to github for Juju bugs, just in ability to navigate between bugs and code more easily, and process impovements. | 09:51 |
| rogpeppe1 | axw: FWIW, it seems to be in the process of tearing down JujuOSEnvSuite, so it's probably not great that it's still running logic from UpgradeSuite :) | 09:52 |
| axw | rogpeppe1: heh, yeah. I think I fixed this somewhere else before. I probably cargo culted before that | 09:53 |
| rogpeppe1 | hmm, no, that's commonMachineSuite | 09:53 |
| rogpeppe1 | axw: it's a pity we can't tell from the stack trace what the embedding type was | 09:54 |
| rogpeppe1 | a fairly trivial change to juju-utils (prompted by the discrepancy between UUID checking in juju/schema and that in utils) https://github.com/juju/utils/pull/137 | 10:07 |
| TheMue | natefinch: +1 | 10:07 |
| rogpeppe1 | there are various UUIDs in the juju tests that fail the more stringent check. I don't really see why they should. e.g. 2d02eeac-9dbb-11e4-89d3-123b93f75cba | 10:08 |
| rogpeppe1 | TheMue: I think it was your code originally | 10:08 |
| rogpeppe1 | TheMue: want to take a look? | 10:08 |
| TheMue | rogpeppe1: yes, any pointers to failing tests? | 10:12 |
| rogpeppe1 | TheMue: I changed environs/config to use utils.IsValidUUID rather than schema.UUID. That caused some tests to fail (try grepping for the above UUID in the juju code) | 10:14 |
| rogpeppe1 | TheMue: I think it's reasonable that the two checks should be aligned, and I don't see any particular reason to forbid non version 4 UUIDs from being parsed | 10:14 |
| * rogpeppe1 thinks the whole notion of "versioned" uuids is weird | 10:16 | |
| === rogpeppe1 is now known as rogpeppe | ||
| TheMue | rogpeppe: I know you're no friends of UUIDs ;) | 10:17 |
| rogpeppe | TheMue: i like UUIDs a lot :) | 10:17 |
| rogpeppe | TheMue: it's just the RFC i have issues with | 10:18 |
| TheMue | rogpeppe: the versions simply express how they are produced or better which information are part of the UUID generation, e.g MAC addresses | 10:18 |
| rogpeppe | TheMue: ... which we never use | 10:18 |
| TheMue | rogpeppe: IMHO an IsValidUUID should allow all versions, not only v4. | 10:19 |
| rogpeppe | TheMue: i guess it might be useful to have a Meta method on UUID that returns information on the UUID by looking at the version | 10:19 |
| rogpeppe | TheMue: but most people these days just use /dev/random so it wouldn't be that useful | 10:19 |
| * TheMue is currently happy, workers produce machine noise from two sides of the house.*aaaargh* | 10:20 | |
| TheMue | rogpeppe: yeah, v4 is the most simple one | 10:20 |
| rogpeppe | TheMue: thanks. i'd appreciate your LGTM on the review if you think it looks reasonable. | 10:21 |
| === brandon is now known as web | ||
| TheMue | rogpeppe: my own lib (https://godoc.org/github.com/tideland/goas/v2/identifier) produces v1, v3, v4, and v5 | 10:22 |
| rogpeppe | TheMue: the only reason I can think of for versioning UUIDs is so you can extract metadata (e.g. the mac address) from the UUID afterwards. i totally don't see the point in version 3 or 5, which don't even preserve the hash | 10:26 |
| mup | Bug #1461871 was opened: worker/diskmanager sometimes goes into a restart loop due to failing to update state <storage> <juju-core:Triaged by axwalk> <https://launchpad.net/bugs/1461871> | 10:26 |
| rogpeppe | TheMue: anyway, old argument, best not restarted :) | 10:27 |
| TheMue | rogpeppe: I have to admit I never looked into the history of UUIDs and why those versions have been created. I only implemented them. *lol* | 10:28 |
| rogpeppe | TheMue: there are many standards that aren't worth implementing :) | 10:29 |
| TheMue | s/standards/code/ | 10:29 |
| TheMue | hmm, and are to is. or does a plural of code makes sense? | 10:30 |
| TheMue | rogpeppe: btw, LGTM | 10:34 |
| rogpeppe | TheMue: i'd probably say "there is much code that isn't worth implementing" | 10:34 |
| rogpeppe | TheMue: thanks | 10:34 |
| TheMue | rogpeppe: yeah, I've seen huge "enterprise systems" containing tons of never called code. but nobody ever removed it and so those systems got more and more unmaintainable. | 10:35 |
| jam | jam | 10:48 |
| rogpeppe | anyone know why state.State.ForEnviron dials mongo rather than just doing a Session.Copy ? | 10:51 |
| thumper | rogpeppe: because I don't understand mongo | 10:57 |
| thumper | :) | 10:58 |
| rogpeppe | thumper: :) | 10:58 |
| thumper | if Copy works, happy to change | 10:58 |
| rogpeppe | thumper: it should do, unless i've misunderstood what's going on | 10:58 |
| thumper | rogpeppe: you probably haven't | 10:58 |
| rogpeppe | thumper: (which is very likely as i'm just skimming through code trying to understand why tests are failing) | 10:58 |
| rogpeppe | thumper: BTW, in case you missed it: http://reviews.vapour.ws/r/1853/ | 11:00 |
| thumper | not looked sorry | 11:00 |
| rogpeppe | thumper: it's the start of what i discussed with you the other night | 11:03 |
| thumper | ack | 11:03 |
| mup | Bug #1461888 was opened: Units stuck in agent-state: down state <juju-core:New> <https://launchpad.net/bugs/1461888> | 11:08 |
| mup | Bug #1461889 was opened: don't turn invalid SetAddresses calls into Assert-only txns <tech-debt> <juju-core:Triaged by jameinel> <https://launchpad.net/bugs/1461889> | 11:08 |
| mup | Bug #1461890 was opened: leadership unreliable in HA <juju-core:Triaged> <https://launchpad.net/bugs/1461890> | 11:08 |
| mup | Bug #1461888 changed: Units stuck in agent-state: down state <juju-core:New> <https://launchpad.net/bugs/1461888> | 11:20 |
| mup | Bug #1461889 changed: don't turn invalid SetAddresses calls into Assert-only txns <tech-debt> <juju-core:Triaged by jameinel> <https://launchpad.net/bugs/1461889> | 11:20 |
| mup | Bug #1461890 changed: leadership unreliable in HA <juju-core:Triaged> <https://launchpad.net/bugs/1461890> | 11:20 |
| mup | Bug #1461888 was opened: Units stuck in agent-state: down state <juju-core:New> <https://launchpad.net/bugs/1461888> | 11:26 |
| mup | Bug #1461889 was opened: don't turn invalid SetAddresses calls into Assert-only txns <tech-debt> <juju-core:Triaged by jameinel> <https://launchpad.net/bugs/1461889> | 11:26 |
| mup | Bug #1461890 was opened: leadership unreliable in HA <juju-core:Triaged> <https://launchpad.net/bugs/1461890> | 11:26 |
| perrito666 | davecheney: from the doc on rand I understand that using just rand.Intn will produce a deterministic result, which I dont really want | 11:37 |
| davecheney | perrito666: do you mean it won't be seeded | 11:54 |
| perrito666 | well I dont feel the doc is not all that clear, but if it behaves as I assumed it does it will use always the same seed | 11:57 |
| perrito666 | which should return a deterministic sequence of numbers on each run of juju | 11:58 |
| perrito666 | I might have just missunderstood | 11:58 |
| davecheney | perrito666: i think juju should see the rng on startup | 11:59 |
| perrito666 | s/see/seed? | 12:00 |
| katco | wwitzel3: ericsnow: jam and i running a little late, brt | 12:01 |
| wwitzel3 | katco: rgr | 12:06 |
| === kadams54-away is now known as kadams54_ | ||
| rogpeppe | here's a fix for an intermittent test failure in worker/envworkermanager; reviews appreciated please: https://github.com/juju/juju/pull/2491 | 12:45 |
| perrito666 | axw: around? | 13:01 |
| axw | perrito666: hey, lurking | 13:01 |
| perrito666 | axw: have time for a short question? (sorry I know its late there) | 13:01 |
| axw | perrito666: sure, what's up? | 13:02 |
| perrito666 | I privmessaged it | 13:02 |
| mup | Bug #1461957 was opened: Does not use security group ids <ci> <juju-core:Triaged> <https://launchpad.net/bugs/1461957> | 13:48 |
| mup | Bug #1461959 was opened: serverSuite teardown fails <ci> <intermittent-failure> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1461959> | 13:48 |
| mup | Bug #1461961 was opened: UniterSuite teardown fails <ci> <intermittent-failure> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1461961> | 13:48 |
| perrito666 | its a shame go sees go go as a typo | 13:51 |
| mup | Bug #1461957 changed: Does not use security group ids <ci> <juju-core:Triaged> <https://launchpad.net/bugs/1461957> | 13:54 |
| mup | Bug #1461959 changed: serverSuite teardown fails <ci> <intermittent-failure> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1461959> | 13:54 |
| mup | Bug #1461961 changed: UniterSuite teardown fails <ci> <intermittent-failure> <test-failure> <juju-core:Invalid> <https://launchpad.net/bugs/1461961> | 13:54 |
| mup | Bug #1461957 was opened: Does not use security group ids <ci> <juju-core:Triaged> <https://launchpad.net/bugs/1461957> | 13:57 |
| mup | Bug #1461959 was opened: serverSuite teardown fails <ci> <intermittent-failure> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1461959> | 13:57 |
| mup | Bug #1461965 was opened: UserSuite setup fails <ci> <unit-tests> <juju-core:Incomplete> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1461965> | 13:57 |
| wwitzel3 | katco: will be about 3 minutes behind standup | 13:59 |
| katco | wwitzel3: k np | 14:02 |
| mup | Bug #1461968 was opened: TestLXCProvisionerObservesConfigChanges fails <ci> <intermittent-failure> <test-failure> <juju-core:New> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1461968> | 14:21 |
| mup | Bug #1461969 was opened: TestDialAgain fails <ci> <intermittent-failure> <test-failure> <juju-core:Incomplete> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1461969> | 14:21 |
| mup | Bug #1461993 was opened: support using an existing vpc <juju-core:New> <https://launchpad.net/bugs/1461993> | 15:03 |
| rogpeppe | jam: it looks like you're ocr today? if so, here are two small juju-core fixes for you, both fixing flaky tests: http://reviews.vapour.ws/r/1859/, http://reviews.vapour.ws/r/1858/ | 15:09 |
| rogpeppe | anyone else: reviews much appreciated. the first one is just a one line fix. | 15:09 |
| jam | rogpeppe: well, technically I'm 2 hours past my EOD but I happen to be around so I'll give it a look | 15:11 |
| jam | first one LGTM | 15:12 |
| rogpeppe | jam: thanks :_ | 15:12 |
| rogpeppe | :) | 15:12 |
| jam | rogpeppe: for http://reviews.vapour.ws/r/1858/diff/# | 15:23 |
| jam | I feel like "errors.Cause(err) == tomb.ErrDying" isn't right. | 15:23 |
| rogpeppe | jam: oh yes? | 15:23 |
| jam | I *feel* like that should be "!= tomb.ErrDying" | 15:23 |
| jam | rogpeppe: why would we only pass up ErrDying if the underlying runner dies? | 15:24 |
| rogpeppe | jam: funnily enough i had it that way originally, but it's wrong | 15:24 |
| rogpeppe | jam: luckily i wrote the test :) | 15:24 |
| jam | rogpeppe: so I think the test you added is handled by go func() { m.tomb.Kill(m.runner.Wait()) } | 15:24 |
| jam | that passes whatever Wait() returns immediately to tomb, doesn't it? | 15:24 |
| rogpeppe | jam: but that can happen at any time in the future | 15:25 |
| rogpeppe | jam: there's no guarantee that it happens before tomb.Done is called | 15:25 |
| jam | sure | 15:25 |
| jam | rogpeppe: but I don't see how the concrete error gets returned from loop() | 15:26 |
| jam | if it came from runner then | 15:26 |
| jam | rogpeppe: ah, you're checking retErr | 15:26 |
| jam | I'm not a big fan of secret named funcs | 15:26 |
| rogpeppe | jam: yes, i wondered if you hadn't noticed that | 15:26 |
| jam | return vars | 15:26 |
| rogpeppe | jam: it's not *that* secret :) | 15:27 |
| jam | so only if the current return reason is ErrDying then override with the error from m.runner | 15:27 |
| rogpeppe | jam: yes | 15:27 |
| jam | rogpeppe: I feel like that is actually the responsibility of tomb.IsFatal and tomb.IsMoreImportant | 15:27 |
| rogpeppe | jam: if you've got a better suggestion for how to phrase it, please tell | 15:28 |
| jam | rogpeppe: shouldn't we just pass m.tomb.Kill() the value of m.runner always? | 15:28 |
| rogpeppe | jam: tomb doesn't have either of those things AFAIR | 15:28 |
| jam | and then loop() returns and passes that in as well? | 15:28 |
| rogpeppe | jam: yeah, that's probably better | 15:29 |
| jam | rogpeppe: k. so the thing I saw was m.runner having the comparison checkers | 15:29 |
| jam | rogpeppe: but Tomb also knows about error priority | 15:29 |
| jam | at least, it knows how to treat ErrDying vs a real error. | 15:29 |
| rogpeppe | jam: yes, w.r.t. other errors > ErrDying | 15:30 |
| jam | rogpeppe: do we have to unwrap our error before passing it to tomb.Kill? | 15:30 |
| jam | given that you are using errors.Cause() | 15:30 |
| jam | we can't pass an errors.Cause() style error directly to tomb.Kill() | 15:30 |
| rogpeppe | jam: that's not actually necessary | 15:30 |
| jam | if Cause() == ErrDying but *err* != ErrDying then tomb doesn't work right. | 15:30 |
| rogpeppe | jam: i just tend to avoid direct error == tests | 15:31 |
| jam | rogpeppe: so new review, LGTM though maybe we'd rather just do m.tomb.Kill(m.runner.Wait()) as we do elsewhere rather than reimplementing that check. | 15:33 |
| jam | rogpeppe: hm... | 15:33 |
| jam | maybe not | 15:33 |
| rogpeppe | jam: ha, it should actually probably be done in the caller function, around line 36 | 15:34 |
| jam | rogpeppe: as if we are talking to m.tomb.Kill() first | 15:34 |
| jam | then we take priority | 15:34 |
| rogpeppe | jam: i'm thinking we probably want the return value from m.runner to take precedence | 15:34 |
| jam | rogpeppe: defer func() {m.runner.Kill(); m.tomb.Kill(m.runner.Wait())} ? | 15:34 |
| jam | rogpeppe: I don't know this func that well. ATM loop() sets the value first | 15:35 |
| jam | And arguably if m.envHasUUID() or whatever fails we're in a state that it doesn't really matter what m.runner returns. | 15:35 |
| jam | sorry m.envHasChanged(uuid) | 15:35 |
| jam | rogpeppe: so what you have is ok, moving into the New* func sounds slightly better. | 15:36 |
| jam | ordering of errors sounds like it is going to get us into trouble unless people can concretely say that they need this error over that one. | 15:37 |
| jam | but if envHasChanged returns an error we are likely in a state where m.runner can't say real things anyway. | 15:37 |
| rogpeppe | jam: it's always an interesting issue | 15:37 |
| rogpeppe | jam: my thought is that m.runner contains the meatiest stuff | 15:37 |
| rogpeppe | jam: so we're much more likely to be interested in that error | 15:38 |
| rogpeppe | jam: (i wish that tomb logged errors when it threw them away) | 15:38 |
| jam | rogpeppe: agreed about tomb | 15:38 |
| rogpeppe | jam: aside: the default value of GOMAXPROCS has changed in go tip, so it's finding these issues more consistently | 15:39 |
| rogpeppe | fwereade: do you have any idea what the root cause is behind the intermittent worker/uniter test failures? | 15:42 |
| rogpeppe | fwereade: e.g. FAIL: uniter_test.go:875: UniterSuite.TestUniterUpgradeConflicts | 15:42 |
| rogpeppe | util_test.go:726: | 15:42 |
| rogpeppe | c.Fatalf("never reached desired status") | 15:42 |
| rogpeppe | jam: after experimenting both ways, i think it looks marginally nicer keeping the defer inside the loop function. then the outer level is the classic defer done, kill(loop) idiom | 15:44 |
| natefinch | rogpeppe: I'm so glad they changed the default of GOMAXPROCS. | 15:44 |
| fwereade | rogpeppe, not offhand -- status-setting has changed lately, though | 15:44 |
| rogpeppe | natefinch: +1 | 15:45 |
| rogpeppe | natefinch: i'm so glad they sped up the scheduler so much :) | 15:45 |
| natefinch | rogpeppe: hah yeah, that too. Though honestly, I think I would have set it to NumCPUs anyway, since it was always surprising to people when it was not (and it would have meant they find more race conditions if someone did set GOMAXPROCS). | 15:46 |
| rogpeppe | fwereade: here's the test output i saw (excluding log messages) FWIW: http://paste.ubuntu.com/11567979/ | 15:46 |
| rogpeppe | jam: i've changed it - PTAL | 15:48 |
| jam | rogpeppe: https://docs.google.com/document/d/1At2Ls5_fhJQ59kDK2DFVhFu3g5mATSXqqV5QrxinasI/edit?pli=1 those GOMAXPROCS benchmarks look really good in 1.5 | 15:48 |
| rogpeppe | jam: yeah, the overhead's more or less gone away | 15:49 |
| fwereade | rogpeppe, hmm, that looks like we're not clearing the resolved flag | 15:49 |
| fwereade | rogpeppe, I always worried that would be racy... :/ | 15:49 |
| * fwereade thinks he might be able to see it -- util_test.go:978 looks like exactly the sort of hack that'd be rendered unstable by status changes | 15:52 | |
| jam | rogpeppe: lgtm | 15:52 |
| rogpeppe | jam: ta! | 15:52 |
| * jam leaves to go play with my son | 15:52 | |
| fwereade | rogpeppe, would you point wallyworld at it please? | 15:52 |
| rogpeppe | jam: enjoy! | 15:52 |
| rogpeppe | fwereade: at the test failure? | 15:52 |
| fwereade | rogpeppe, yeah, I'm blithely assertinng that he did status stuff and should be in a good position to track it down | 15:53 |
| fwereade | rogpeppe, or possibly perrito666? ^^ | 15:53 |
| rogpeppe | ha, i bet this is the same issue | 15:57 |
| rogpeppe | https://bugs.launchpad.net/juju-core/+bug/1448308 | 15:57 |
| mup | Bug #1448308: Skipped TestUniterUpgradeConflicts on ppc64 <skipped-test> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1448308> | 15:57 |
| rogpeppe | coretesting.SkipIfPPC64EL(c, "lp:1448308") | 15:57 |
| rogpeppe | that's surely bogus | 15:58 |
| rogpeppe | fwereade: it would be nice to have comments on the uniter test primitives. for example, what are the various fields in the waitUnitAgent meant to imply? | 16:03 |
| rogpeppe | fwereade: what does a resolved status of "no hooks" imply? | 16:07 |
| rogpeppe | sorry, no-hooks | 16:07 |
| rogpeppe | oh i see, ignore me | 16:08 |
| voidspace | dimitern: http://reviews.vapour.ws/r/1860/ | 16:10 |
| mgz_ | oh, forgot to propose branch... | 16:13 |
| perrito666 | sorry I was afk, not having the best health day | 16:14 |
| mgz_ | rogpeppe: just for you, http://reviews.vapour.ws/r/1861 | 16:14 |
| perrito666 | fwereade: rogpepe, what's going on? | 16:14 |
| rogpeppe | mgz_: i think a fix for those is already landing | 16:14 |
| mgz_ | doh, this is why I shouldn't forget branches | 16:15 |
| rogpeppe | mgz_: as part of https://github.com/juju/juju/pull/2487/files | 16:15 |
| rogpeppe | mgz_: although... i can't land that quite yet as i can't get tests to pass | 16:16 |
| rogpeppe | mgz_: so LGTM, go for it | 16:16 |
| mgz_ | rogpeppe: sure thing | 16:16 |
| dimitern | voidspace, looking, but also in a call so it might take some time | 16:16 |
| mgz_ | rogpeppe: I actually did that change after you said in irc but forgot about it... | 16:16 |
| voidspace | dimitern: np, I'm working on the higher level stuff anyway | 16:17 |
| perrito666 | rogpeppe: fwereade do you still need my help? that particular test suite is a pain to follow | 16:20 |
| natefinch | ericsnow: I presume ProcessInfo.Status is supposed to be an enum? i.e., you can't just put whatever string you want in there | 16:20 |
| ericsnow | natefinch: pretty much | 16:20 |
| rogpeppe | perrito666: i'm seeing a consistently reproducible failure in that test | 16:20 |
| rogpeppe | perrito666: and i don't really want to get sidelined into fixing it | 16:21 |
| natefinch | ericsnow: I'm going to leave a comment that says we should make it a numeric enum. Making it a string just makes it less obvious it's supposed to be an enum | 16:21 |
| perrito666 | rogpeppe: and this only happens in ppc | 16:22 |
| perrito666 | ? | 16:22 |
| rogpeppe | perrito666: no, this is on my normal laptop | 16:23 |
| rogpeppe | perrito666: although i am running with go tip | 16:23 |
| rogpeppe | perrito666: i'm sure it's just an inherent problem with the test though | 16:23 |
| perrito666 | rogpeppe: yes, that test is as brittle as a crystal hamer | 16:25 |
| perrito666 | hammer* | 16:25 |
| perrito666 | well not really brittle, it just makes a very good job hiding real issues | 16:25 |
| rogpeppe | perrito666: verifyWaitingUpgradeError in particular seems very handwavy | 16:26 |
| perrito666 | rogpeppe: this is master tip rigt? | 16:26 |
| rogpeppe | perrito666: yes | 16:27 |
| * perrito666 runs the test | 16:27 | |
| rogpeppe | perrito666: try running it with GOMAXPROCS=4 | 16:27 |
| rogpeppe | perrito666: and run it a few times. | 16:28 |
| rogpeppe | perrito666: (best to use go test -c and then run the test binary directly) | 16:28 |
| perrito666 | oh it is one of those bugs | 16:28 |
| rogpeppe | perrito666: yeah, it's definitely a race | 16:29 |
| natefinch | nick natefinch-afk | 16:29 |
| dimitern | voidspace, reviewed | 16:55 |
| voidspace | dimitern: thanks | 17:01 |
| voidspace | dimitern: cool, not much to do - thanks | 17:02 |
| dimitern | voidspace, well, the code looks solid :) | 17:09 |
| === kadams54_ is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54_ | ||
| perrito666 | hey I cannot make it to today's meeting I have to take my wife to the dentist, sorry ppl | 17:50 |
| === kadams54_ is now known as kadams54-away | ||
| alexisb | perrito666, team call | 18:04 |
| alexisb | voidspace, team call if you are still around | 18:04 |
| alexisb | natefinch, team call | 18:04 |
| perrito666 | alexisb: as I said abvove: <perrito666> hey I cannot make it to today's meeting I have to take my wife to the dentist, sorry ppl | 18:05 |
| === web is now known as Web | ||
| Web | https://jujucharms.com/static/img/jujudocs/1.23/getting_started-aws_security.png <-- time to change this I think. There is a emphasis on iAM profiles now. Need to run through the process of using one first. | 18:10 |
| natefinch | alexisb: oops, sorry coming | 18:13 |
| === kadams54-away is now known as kadams54_ | ||
| mgz_ | ericsnow: I have a proposed fix, but I don't think I actually have a bitbucket account | 18:47 |
| ericsnow | mgz_: k | 18:48 |
| mgz_ | draft.rich_text = True | 18:48 |
| ericsnow | mgz_: ah | 18:48 |
| mgz_ | but there's also a description_rich_text | 18:49 |
| mgz_ | just tracking down if that's a subset or not | 18:49 |
| mgz_ | aha, just rich_text is deprecated | 18:53 |
| mgz_ | so, description_rich_text and testing_done_rich_text are the new way | 18:53 |
| katco | natefinch: 1:1 time | 19:07 |
| natefinch | katco: dang, sorry | 19:07 |
| natefinch | katco: coming | 19:07 |
| katco | no worries | 19:08 |
| perrito666 | are there no tests for modes code? | 20:12 |
| natefinch | perrito666: we don't test code that rhymes | 20:15 |
| perrito666 | natefinch: not fun | 20:16 |
| natefinch | perrito666: better run | 20:17 |
| perrito666 | you know... you are lucky to be so far away :p | 20:18 |
| natefinch | but I do, so I can rhyme all day :) | 20:19 |
| * perrito666 shops for a ticket to boston | 20:19 | |
| * perrito666 shops for a clue by four | 20:20 | |
| natefinch | sorry, A/C man at the door. Sounds like he's not going to make us poor. | 20:33 |
| thumper | morning | 20:46 |
| natefinch | morning | 20:46 |
| thumper | how's it going nate? | 20:47 |
| natefinch | thumper: not bad... getting our A/C fixed.... which is good, because we thought it needed to be replaced until we got a second opinion. | 20:51 |
| natefinch | thumper: so, probably going to cost us ~$500 instead of ~$10,000 | 20:52 |
| thumper | I bet you're pleased about that | 20:52 |
| natefinch | immensely. We definitely had not planned on spending $10,000 on the A/C this summer. | 20:54 |
| natefinch | unrelated, I'm trying to figure out a way to get github to be a satisfactory bug tracker for us, because i'm tired of not having integration with the code. | 20:54 |
| thumper | natefinch: nope, nope, nope | 20:55 |
| natefinch | heh | 20:55 |
| natefinch | thumper: what's wrong with github's bug tracker? | 20:55 |
| thumper | natefinch: I'm not even going to get started | 20:56 |
| thumper | I don't have long enough | 20:56 |
| natefinch | thumper: haha.... that's ok, me neither | 20:56 |
| * mgz_ hugs thumper | 20:56 | |
| natefinch | me either? me neither? I guess I should say "I also do not have time" | 20:56 |
| mup | Bug #1462097 opened: Bootstrap error logging needs to be more descriptive <juju-core:New> <https://launchpad.net/bugs/1462097> | 21:11 |
| === weblife is now known as Web | ||
| === kadams54_ is now known as kadams54-away | ||
| === Beret- is now known as Beret | ||
| axw | perrito666: worker/uniter/uniter_test.go tests the modes indirectly | 23:14 |
| axw | they're more functional style tests than unit tests | 23:14 |
| wallyworld | axw: anastasiamac: perrito666: just fiishing a meetig, me there soon | 23:15 |
| axw | sure | 23:15 |
| anastasiamac | k | 23:15 |
| perrito666 | axw: well yes, I am trying to figure out how to fit my test | 23:17 |
| axw | wallyworld anastasiamac: I raised https://bugs.launchpad.net/juju-core/+bug/1462146 | 23:31 |
| mup | Bug #1462146: cmd/juju/storage: "add" fails to dynamically add filesystem for storage <storage> <juju-core:New> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1462146> | 23:31 |
| wallyworld | ty | 23:31 |
| anastasiamac | axw: tyvm :D | 23:31 |
| wallyworld | axw: anastasiamac: btw, i've started to merge 1.24 into master, *lots* of conflicts so i'll see how i go. might need to cherry pick individual commits if it's too hard | 23:44 |
| axw | wallyworld: thank you | 23:44 |
| anastasiamac | wallyworld: sure! just say the word :D | 23:44 |
| mup | Bug #1462146 opened: cmd/juju/storage: "add" fails to dynamically add filesystem for storage <storage> <juju-core:New> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1462146> | 23:44 |
| davecheney | mwhudson: would it short circuit the debian debate if I proposed a change which just added the words we wanted to that page ? | 23:54 |
| mwhudson | davecheney: quite possibly, yes | 23:57 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
