| davechen1y | thumper: menn0 axw https://github.com/juju/testing/pull/85 | 00:15 |
|---|---|---|
| davechen1y | ^ for skipping go 1.5 failing tests | 00:15 |
| mup | Bug #1520380 opened: worker/provisioner: unit tests fail on xenial <juju-core:New> <https://launchpad.net/bugs/1520380> | 00:19 |
| thumper | davechen1y: kk | 00:21 |
| menn0 | thumper: ship it | 00:21 |
| thumper | davechen1y: why use the testing change instead of just using the go version in the code? | 00:22 |
| thumper | davechen1y: what does it buy us? | 00:22 |
| davechen1y | thumper: same as https://github.com/juju/testing/pull/84 | 00:23 |
| davechen1y | we're probably going to need to do it in a bunch of places | 00:24 |
| davechen1y | using build constraints requires at least 5 files | 00:24 |
| davechen1y | better to put it in one place, than replicate those turds across every package we have to skip conditionally | 00:24 |
| thumper | ok | 00:24 |
| thumper | good enough for me | 00:24 |
| davechen1y | also, when we're done with this, it's one place to delete | 00:25 |
| * thumper nods | 00:25 | |
| menn0 | davechen1y: I know I said Ship it, but one annoyance with this scheme is that we have to keep adding files as new Go versions come out | 00:25 |
| davechen1y | menn0: yes, and no | 00:25 |
| menn0 | Parsing the output of https://golang.org/pkg/runtime/#Version is another option | 00:25 |
| davechen1y | you only need to add a new file if you want to identify that version of go | 00:25 |
| davechen1y | go 1.6 will identify as go 1.5 under this scheme | 00:25 |
| davechen1y | and also, when/if that happens, it happens in one place | 00:25 |
| davechen1y | and when we're done with this aweful hack, we can delete it in one palce | 00:26 |
| davechen1y | and when we're done with this aweful hack, we can delete it in one place | 00:26 |
| menn0 | davechen1y: i'm fine with the approach, just wondering if using runtime.Version would be slightly better. it's fine the way it is. | 00:27 |
| davechen1y | it would be messier | 00:27 |
| davechen1y | parsing, yeah easy enough | 00:27 |
| davechen1y | but we'd lose the constant nature of these constnts | 00:27 |
| davechen1y | you'd get a float64, and get shat on by precision | 00:27 |
| davechen1y | equals may not work | 00:27 |
| davechen1y | doing it this way gives us equality between ideal constants | 00:28 |
| davechen1y | if testing.GOVERSION == 1.5 is a constant expression | 00:28 |
| davechen1y | i suspect if we did it by parsing runtime.Version, we'd end up _also_ defining a bunch of constants then returning those constnat values | 00:29 |
| davechen1y | which would also be forced throught float64 coercion, blah blah | 00:29 |
| davechen1y | this is less worse doing it this way | 00:30 |
| davechen1y | and if it feels a bit icky, that's good | 00:30 |
| davechen1y | we should be skiping all this crap | 00:30 |
| davechen1y | https://github.com/juju/juju/pull/3839 | 00:34 |
| davechen1y | ^ trivial review | 00:34 |
| davechen1y | axw: mgz_ http://juju-ci.vapour.ws/job/run-unit-tests-xenial-amd64/129/consoleFull | 00:34 |
| davechen1y | ^ what job triggers this build, is it build-revisions ? | 00:34 |
| mgz_ | it's not triggered in jenkins, the comment at the top is what tells lp:ci-director to run it, as soon as build-revison happens | 00:35 |
| mgz_ | from last night though, if you want to re-run a job | 00:36 |
| mgz_ | just login as developer per creds in lp:cloud-city | 00:36 |
| mgz_ | "build now" and give it the build revision job number (which serves as an index for all tests from one run) | 00:37 |
| davechen1y | ok, so if I land a fix for the xenial test failure, i should wait for build-revision to pick it up | 00:37 |
| mgz_ | yeah, to run a new revison just wait for it to go through ci as normal | 00:37 |
| davechen1y | mgz_: I noticed that if I kill a -race job that is working on a branch which I know will never pass | 00:39 |
| davechen1y | ci just runs it again | 00:39 |
| davechen1y | is there any way to avoid this ? | 00:39 |
| davechen1y | it wastes 3 hours every time it chews on some branch that will never pass | 00:39 |
| mgz_ | davechen1y: change failure-threshhold to 1 | 00:40 |
| mgz_ | issue with that being... the tests do randomly fail sometimes | 00:40 |
| davechen1y | mgz_: meh | 00:40 |
| mgz_ | so we get curses which retesting will 'cure; | 00:40 |
| davechen1y | if they fail sometimes, i'll comment 'em out | 00:41 |
| mgz_ | davechen1y: what you can do for that manual thing | 00:41 |
| mgz_ | is change it to one then... hm.. actually, you'd need to change it back after the entire run finished, which would suck | 00:41 |
| mgz_ | davechen1y: the problem with the just0skip-it approach | 00:42 |
| davechen1y | hmm | 00:42 |
| davechen1y | it sounds like I should just live with it | 00:42 |
| davechen1y | this problem will solve itself soon enough when feature branches rebase | 00:42 |
| mgz_ | well, problem one is we've been historically bad at fixing unreliable tests, skipping them just makes the problem easier to ignore | 00:42 |
| davechen1y | mgz_: no argument there | 00:43 |
| mgz_ | davechen1y: we can chose to not run race vs feature branches... well, that's bad long term, but we could blacklist a few now | 00:43 |
| mgz_ | anyway, problem two with skipping tests is sometimes it's like, "this entire suite is flakey" | 00:43 |
| davechen1y | mgz_: nah, this problem will solve itself soon enough | 00:43 |
| mgz_ | losing *all* our uniter coverage say, is bad | 00:43 |
| davechen1y | mgz_: no arguemnt there | 00:44 |
| mgz_ | we've shipped bad code in the past because tests have been ignored | 00:44 |
| davechen1y | i'm stuck between a rock and a hard place | 00:44 |
| mgz_ | indeed | 00:44 |
| davechen1y | thumper: do you have any comment on this position ? | 00:44 |
| thumper | how far back do I need to read? | 00:44 |
| davechen1y | 2 minutes | 00:45 |
| davechen1y | 4 at most | 00:45 |
| davechen1y | mgz_: rightly points out that historically skipping a failing package takes the issue off the radar | 00:45 |
| mgz_ | thumper: I write concisely and with clarity, surely reading isn't that much of a chore for you ;_; | 00:45 |
| davechen1y | i don't disagree, but historically fixing flakey tests, especially races as been an uphill battle | 00:45 |
| davechen1y | meaning we have no race coverage, so nothing prevening new races coming in | 00:45 |
| mgz_ | I think your approach on making -race useful was good | 00:46 |
| thumper | hmm... | 00:46 |
| thumper | I understand the races one | 00:46 |
| thumper | as people kept adding them too frequently | 00:46 |
| mgz_ | the tests just took way too long and had too much random output to be good without culling a bunch | 00:46 |
| thumper | I'm guessing that the wily failures are more limited? | 00:46 |
| thumper | as in we know which ones fail often? | 00:46 |
| davechen1y | xenial has 1 failure | 00:47 |
| davechen1y | worker/provisioner | 00:47 |
| mgz_ | welll... there's a few obvious bugs, like lxd borked a few tests | 00:47 |
| davechen1y | skipping that for go 1.5 gets us a voting go 1.5 build | 00:47 |
| mgz_ | and there's a few new intermittent failures, which... could be a number of things | 00:47 |
| davechen1y | which is crucial for upgrading to go 1.5 | 00:47 |
| thumper | what's the difference between failures on xenial and wily? | 00:47 |
| davechen1y | no idea | 00:47 |
| davechen1y | haven't looked at wily | 00:47 |
| mgz_ | and we get, possibily just a different symptom of, mongodb related failure | 00:47 |
| mgz_ | they're much the same | 00:47 |
| mgz_ | the package set hasn't really diverged, and it's all go 1.5 | 00:48 |
| * davechen1y is not touching the mongodb debt clusterfuck | 00:48 | |
| mgz_ | if it's just the mongodb panic we can probably contain that | 00:48 |
| mgz_ | run three times, hope, etc, as previously | 00:48 |
| thumper | davechen1y: how about just fixing the test rather than skipping it if it is just one? | 00:49 |
| thumper | my main concern is that with 1.5 | 00:49 |
| thumper | if we are skipping a lot of tests | 00:50 |
| thumper | and losing coverage | 00:50 |
| thumper | we are losing faith that the code works | 00:50 |
| davechen1y | thumper: this is worker/provisioner | 00:50 |
| davechen1y | its riddled with timing issues | 00:50 |
| davechen1y | there have been bugs open against that packge for near 2 years | 00:50 |
| davechen1y | so clearly there is no priority in fixing it | 00:51 |
| thumper | so my point stands, if go 1.5 is more likely to cause problems in the provisioner due to the way things run | 00:52 |
| thumper | I'd rather have a failing test than a skipped test | 00:52 |
| thumper | my stance is generally this: | 00:53 |
| thumper | skipping shitty tests is one thing | 00:53 |
| thumper | but skipping good tests that show bad code is wrong | 00:53 |
| mgz_ | davechen1y: aside, I just added some code to the -race job config to show skipping particilar feature branches you know will fail, feel free to fill in branch names | 00:53 |
| davechen1y | you're fine with skipping packages with known data races, but you're concerned about skipping a package with logical timing issues ?!? | 00:53 |
| davechen1y | mgz_: thanks! | 00:53 |
| thumper | davechen1y: well... | 00:54 |
| thumper | the tests pass.. FSVO pass | 00:54 |
| davechen1y | you cannot choose which is the lesser issue | 00:54 |
| thumper | perhaps it is just a false sense of security | 00:54 |
| davechen1y | you cannot choose which is the lesser evil | 00:54 |
| davechen1y | every package I skip is tracked with an LP issue | 00:54 |
| davechen1y | make them critical blockers if you like | 00:54 |
| thumper | davechen1y: do you have tags on them? | 00:55 |
| thumper | if not, please add some | 00:55 |
| mgz_ | well, I guess the real issue is how close we are to getting stuff actually fixed in races vs 1.5 - I think there's probably only two issues we need looked at for 1.5 to get at least a resonable rate of blue | 00:55 |
| davechen1y | thumper: nope, no tags | 00:55 |
| davechen1y | which tags would you like ? | 00:55 |
| mgz_ | skipped-test | 00:56 |
| mgz_ | is what I have used | 00:56 |
| davechen1y | bzzr | 00:56 |
| davechen1y | bzzt | 00:56 |
| davechen1y | no new tag | 00:56 |
| thumper | how about skipped-test + ci-failure + ci-test name? | 00:56 |
| davechen1y | we have so many tags for "critical", "omg critical", "exploding customer", etc | 00:56 |
| davechen1y | adding another tag is a net negative | 00:56 |
| davechen1y | I think we should use milestones and LP status | 00:56 |
| thumper | a tag for skipped-test is fine | 00:56 |
| davechen1y | i disagree | 00:58 |
| davechen1y | historically tags have proved bad at communicating severity or priority | 00:58 |
| davechen1y | i think we should use LP severity | 00:58 |
| mgz_ | I agree with that, but it's not really a tag thing | 00:58 |
| davechen1y | which will automatically fit into the work that cherylj is doing with her dashboards | 00:58 |
| davechen1y | for example, we already have several tags for data-race | 00:59 |
| mgz_ | the main issue is no one looks at bugs as part of their daily workflow (well, except curtis, and we made him stop) | 00:59 |
| davechen1y | none have proven to motivate people to fix them | 00:59 |
| davechen1y | mgz_: no argument here | 00:59 |
| davechen1y | mgz_: no argument there | 00:59 |
| mgz_ | so, no one was taking bugs off the list of new issues and working on them unless instructed, either by hammer-on-head or by ci going into sirens-mode | 01:00 |
| davechen1y | mgz_: yup, so here is my proposition | 01:01 |
| davechen1y | skip the tests, get the jobs voting, fix the skipped tests | 01:01 |
| davechen1y | we've tried to do it in another order for some time now | 01:01 |
| davechen1y | without success | 01:01 |
| davechen1y | time to try something else | 01:01 |
| davechen1y | note: tests are skiped only in specific conditions, which are only true for a subset of CI builds | 01:01 |
| davechen1y | I am not unilaterally disabling the tests of a packge | 01:02 |
| davechen1y | mgz_: thumper for example, https://github.com/juju/juju/pull/3840/files | 01:02 |
| davechen1y | https://bugs.launchpad.net/juju-core/+bug/1520380 | 01:05 |
| mup | Bug #1520380: worker/provisioner: unit tests fail on xenial <juju-core:Confirmed> <https://launchpad.net/bugs/1520380> | 01:05 |
| axw | davechen1y: I think I have a fix for that test | 01:24 |
| thumper | found another bug | 01:25 |
| thumper | when status should indicate that an agent is down | 01:26 |
| thumper | it only sets the old status | 01:26 |
| thumper | ah this is so f'ed up | 01:28 |
| davechen1y | thumper: where was the log and return nil ? | 01:31 |
| thumper | instance poller | 01:32 |
| thumper | it should be setting the instance status to missing / unknown / down whatever | 01:32 |
| davechen1y | thumper: urgh | 01:33 |
| davechen1y | do you want me to take a pass at fixing that, or are you on it ? | 01:33 |
| davechen1y | axw: sure, if you reckon you have a fix then we can put my PR on the shelf | 01:34 |
| axw | davechen1y: just running all tests now, should have a PR up shortly | 01:34 |
| davechen1y | axw: thanks! | 01:38 |
| thumper | wallyworld: what is up with your hangouts today? | 01:44 |
| wallyworld | nfi | 01:44 |
| wallyworld | i'm here | 01:44 |
| thumper | I can't hear you on the hangout | 01:45 |
| wallyworld | thumper: reconnecting, google has been bad today for me | 01:45 |
| wallyworld | docs as well | 01:45 |
| axw | davechen1y: FYI, http://reviews.vapour.ws/r/3260/ | 01:48 |
| davechen1y | axw: hmm | 01:54 |
| davechen1y | that isn't the same issue | 01:54 |
| davechen1y | https://launchpad.net/bugs/1520380 | 01:54 |
| mup | Bug #1520380: worker/provisioner: unit tests fail on xenial <juju-core:Confirmed> <https://launchpad.net/bugs/1520380> | 01:54 |
| thumper | wallyworld: sorry, giving up on the hangout | 01:55 |
| wallyworld | thumper: yeah, sorry | 01:55 |
| wallyworld | nfi | 01:55 |
| wallyworld | google is shite today | 01:56 |
| thumper | wallyworld: I'm going to write up a small spec re presense properly | 01:56 |
| wallyworld | ok | 01:56 |
| axw | davechen1y: it is, grep for "unknown container type: lxd" | 01:56 |
| davechen1y | fair enough | 01:56 |
| davechen1y | lets commit it and see what happens | 01:56 |
| thumper | axw: do you have a few minutes? wallyworld said you have thought quite a bit about presense already | 01:58 |
| thumper | and I'm right in that head space | 01:58 |
| thumper | axw: no worries if you don't have time | 01:58 |
| axw | thumper: can do, but meant to have a 1:1 with wallyworld in 2mins | 01:58 |
| wallyworld | axw: we can delay our 1:1 | 01:58 |
| axw | ok | 01:58 |
| wallyworld | ping me when free | 01:58 |
| axw | thumper: sure, name a place | 01:58 |
| axw | wallyworld: okey dokey | 01:58 |
| thumper | axw: https://plus.google.com/hangouts/_/canonical.com/presence?authuser=1 | 01:59 |
| axw | wallyworld: ready | 02:14 |
| mgz_ | menn0: I <3 u | 02:16 |
| wallyworld | ok | 02:16 |
| davechen1y | thumper: mwhudson https://groups.google.com/d/topic/golang-dev/y-mlM-XYysk/discussion | 02:16 |
| wallyworld | mgz_: do you want to share a room with him? | 02:16 |
| mgz_ | that may now be dangerous | 02:17 |
| * menn0 is glad we have our own rooms in Oakland | 02:17 | |
| axw | davechen1y: nice :) | 02:17 |
| mwhudson | davechen1y: oh hooray | 02:17 |
| davechen1y | i'm gobsmacked they did it in secrecy | 02:18 |
| mwhudson | yeah, seems insane | 02:18 |
| mwhudson | glad they're talking about it | 02:18 |
| mwhudson | still no link to the source though... | 02:18 |
| menn0 | mgz_: so as well as 1.23 being a plague release, we know that 1.21 is dangerous too (well, only if any upgrade step fails) | 02:19 |
| davechen1y | mwhudson: i do hope they based their work of Go 1.5 or later | 02:19 |
| mgz_ | yeah, we should have apparently just kept the whol odd-numbers-are-bad thing | 02:19 |
| davechen1y | if they used 1.4 | 02:19 |
| mwhudson | davechen1y: ah uh yes | 02:19 |
| davechen1y | that will be, complicated | 02:19 |
| anastasiamac | mgz_: menn0 <3s scotch (probably safer than sharing a room...) | 02:19 |
| menn0 | mgz_: and before you get too excited... the bug I found was created by me :( | 02:21 |
| * menn0 hangs his head in shame | 02:21 | |
| mgz_ | ehehe, that just makes it more amusing | 02:21 |
| anastasiamac | double the scotch then \o/ | 02:21 |
| menn0 | i'm glad it only exists in 1.21.N releases | 02:22 |
| thumper | davechen1y: nice | 02:30 |
| mup | Bug #1520314 changed: Environment not usable after upgrade from 1.21.3 -> 1.25.0 fails with '"cannot retrieve meter status for unit xxx/0: not found"' <sts> <upgrade-juju> <juju-core:Invalid> <juju-core 1.21:Won't Fix by menno.smits> <https://launchpad.net/bugs/1520314> | 02:31 |
| davechen1y | axw: sorry, your change did not fix the problem I am seeing | 02:52 |
| davechen1y | mgz_: the build-revision job appears to be disabled | 02:53 |
| davechen1y | is that how it works, or is that a mistake ? | 02:53 |
| davechen1y | axw: http://paste.ubuntu.com/13522160/ | 02:53 |
| axw | davechen1y: is that from juju-ci, or your machine? | 02:59 |
| davechen1y | my machine | 02:59 |
| davechen1y | but matches exaclty what I see for the xenial build | 02:59 |
| axw | davechen1y: gah, I must have missed something. I'm getting it too | 03:00 |
| axw | looking now | 03:01 |
| davechen1y | ta | 03:03 |
| axw | davechen1y: no, sorry, I just hadn't pulled master. works on my machine ... :/ | 03:03 |
| axw | davechen1y: can you confirm that "instance/container_go13.go" is not in your tree? | 03:03 |
| davechen1y | lucky(~/src/github.com/juju/juju/worker/provisioner) % ls | 03:04 |
| davechen1y | container_initialisation.go export_test.go kvm-broker_test.go logging_test.go lxc-broker_test.go provisioner.go provisioner_test.go | 03:04 |
| davechen1y | container_initialisation_test.go kvm-broker.go logging.go lxc-broker.go package_test.go provisioner_task.go | 03:04 |
| davechen1y | hmm, are we looking at the same thing ? | 03:05 |
| davechen1y | afk for a bit | 03:05 |
| axw | davechen1y: what's in github.com/juju/juju/instance ? | 03:05 |
| axw | ok | 03:05 |
| davechen1y | ok, crisis averted | 03:18 |
| davechen1y | axw: lucky(~/src/github.com/juju/juju/instance) % ls | 03:29 |
| davechen1y | container.go container_test.go instance.go instance_test.go placement.go placement_test.go testing | 03:29 |
| axw | davechen1y: welp, I dunno then. I kicked off a build-revision job anyway | 03:29 |
| thumper | hmm... | 03:57 |
| thumper | wallyworld: I have a few questions about unit agent status | 03:57 |
| wallyworld | ok | 03:57 |
| axw | davechen1y: http://juju-ci.vapour.ws:8080/job/run-unit-tests-xenial-amd64/133/console | 04:01 |
| axw | davechen1y: Finished: SUCCESS | 04:01 |
| davechen1y | weird, it still fails 100% for me | 04:02 |
| axw | davechen1y: only thing I can think of is that the juju/instance package didn't rebuild | 04:03 |
| davechen1y | axw: i have a change coming | 04:07 |
| davechen1y | its not a logic error | 04:08 |
| thumper | laters folks | 04:24 |
| menn0 | wallyworld or axw: http://reviews.vapour.ws/r/3262/ please | 04:50 |
| wallyworld | sure | 04:50 |
| menn0 | wallyworld or axw: no major rush as I'm done for the week | 04:50 |
| menn0 | wallyworld: thanks | 04:51 |
| wallyworld | menn0: first glance, looks like a lot of moving of code? | 04:51 |
| menn0 | wallyworld: yes, and a lot of changes to the moved code | 04:51 |
| wallyworld | \o/ | 04:51 |
| menn0 | wallyworld: the PR and separate commits give more detail | 04:52 |
| wallyworld | ok, will look a little later | 04:52 |
| menn0 | wallyworld: maybe it's easier to look at the individual commits on Github? | 04:52 |
| wallyworld | yeah | 04:52 |
| === akhavr1 is now known as akhavr | ||
| voidspace | dimitern: ping - if you have a minute | 09:03 |
| dimitern | voidspace, sure | 09:04 |
| voidspace | dimitern: so maas does support ipv6 subnets (max of one per interface) | 09:04 |
| dimitern | voidspace, I didn't know it's just one | 09:04 |
| voidspace | dimitern: my PR is done, most of your suggestions are in (tests refactoring and reformatting etc) | 09:04 |
| voidspace | dimitern: according to their ipv6 docs | 09:04 |
| dimitern | voidspace, cheers | 09:04 |
| dimitern | voidspace, I'll have another look before standup | 09:05 |
| voidspace | dimitern: however my code as written won't work with ipv6 - it assumes ipv4 for working out the allocatable range | 09:05 |
| voidspace | dimitern: there's a TODO in there as you suggested | 09:05 |
| voidspace | dimitern: should I fix it (without manual tests as I don't have an ipv6 setup) | 09:05 |
| voidspace | dimitern: or ok to leave it as a TODO for now? | 09:05 |
| dimitern | voidspace, as long as we keep it in mind, a TODO is fine for now I think | 09:06 |
| voidspace | dimitern: cool | 09:06 |
| voidspace | dimitern: I'll let you have another look before I land it then | 09:06 |
| voidspace | dimitern: the only thing I didn't do was the extra work in the dummy provider | 09:06 |
| voidspace | dimitern: I figure we can do that when we actually need it | 09:06 |
| voidspace | it would be unused code until then | 09:06 |
| voidspace | and I have ethical objections to unused code | 09:07 |
| dimitern | voidspace, :) agreed - we can add it later, leaving a TODO there with some of the things we need would be good though - I'll comment | 09:08 |
| dimitern | voidspace, reviewed | 09:33 |
| voidspace | dimitern: subnetIdSet is a map, so ranging over it gets the keys and values not an index and value | 09:35 |
| voidspace | dimitern: and it is a map because the bool value has semantic meaning | 09:35 |
| voidspace | dimitern: if you read the code notFound is *not* the same length as subnetIdSet | 09:35 |
| voidspace | dimitern: we only put in it the not found ones | 09:36 |
| dooferlad | dimitern, voidspace: why are we treating IPv4 and IPv6 addresses differently at all? They can both be represented as uint64. | 09:36 |
| dooferlad | dimitern, voidspace: subnet masks the same | 09:36 |
| voidspace | dimitern: that's true for both your first two comments | 09:36 |
| dimitern | voidspace, ah, I was too quick then :) I'll drop that issue | 09:37 |
| dimitern | voidspace, we should use sets when possible though | 09:38 |
| dimitern | in cases like this | 09:38 |
| voidspace | dooferlad: because the code as written only works with IPv4 and would require rewriting | 09:38 |
| voidspace | dimitern: did you read what I wrote... | 09:38 |
| voidspace | dimitern: about why it is a map | 09:38 |
| dimitern | voidspace, still, the second comment about using a helper for extracting notFound from subnetIdsSet holds | 09:38 |
| dimitern | voidspace, yeah | 09:39 |
| voidspace | dimitern: it needs to be a map | 09:39 |
| dimitern | voidspace, yeah | 09:39 |
| voidspace | dimitern: ok, I can do that | 09:39 |
| voidspace | dooferlad: plus it would need some code to track if it's ipv4 or ipv6 addresses we're using | 09:41 |
| voidspace | dooferlad: as addressing need converting back to the correct format | 09:41 |
| voidspace | dooferlad: it's not hard it's just not zero work | 09:42 |
| dooferlad | voidspace: I mostly bring it up because it isn't hard and by landing this we introduce tech debt. | 09:44 |
| dooferlad | voidspace: that said, I am happy to write a followup. | 09:45 |
| dooferlad | voidspace: (as I am sure you would be) | 09:45 |
| voidspace | dooferlad: cool | 09:45 |
| voidspace | dooferlad: yep, there's a couple of places that use that conversion code that would need changing | 09:47 |
| voidspace | dooferlad: definitely worth doing | 09:47 |
| voidspace | dimitern: that trailing bracket was an oversight :-p | 09:47 |
| voidspace | dimitern: thanks for catching it | 09:47 |
| dooferlad | voidspace: thankfully IPv4ToDecimal is used in four places before your change (other than tests), so killing it with fire is easy | 09:47 |
| dimitern | voidspace, any time :) | 09:48 |
| voidspace | dimitern: changes pushed | 09:51 |
| voidspace | dimitern: it actually needs dooferlad's changes to gomaasapi to land first | 09:51 |
| voidspace | dooferlad: did you get a review for your branch? | 09:51 |
| voidspace | dooferlad: if not I'll look at it | 09:51 |
| dooferlad | voidspace: no, needs a review | 09:51 |
| dimitern | voidspace, cheers - let's land it | 09:51 |
| voidspace | cool | 09:52 |
| voidspace | dooferlad: 1400 lines! | 09:54 |
| voidspace | dooferlad: I'm concerned that the struct approach to serialising sends empty (uninitialised) arrays as null | 09:55 |
| voidspace | dooferlad: which doesn't match maas and will break code that does GetArray() | 09:55 |
| dooferlad | voidspace: happy to run through it with you after the hangout | 09:59 |
| voidspace | frobware: you around? | 10:02 |
| dimitern | fwereade, jam, standup? | 10:02 |
| frobware | dimitern, strike 3 | 10:04 |
| voidspace | dimitern: jam is never around on Friday :-) | 10:04 |
| voidspace | dooferlad: will sync up with you shortly | 10:32 |
| dooferlad | voidspace: cool | 10:32 |
| === beisner- is now known as beisner | ||
| === cppforlife__ is now known as cppforlife_ | ||
| fwereade | straw poll: WaitExpired, WaitForExpiry, WaitUntilExpired? | 11:31 |
| fwereade | mgz, dimitern, frobware, voidspace? ^ | 11:31 |
| dimitern | fwereade, last one | 11:32 |
| fwereade | dimitern, cheers | 11:33 |
| frobware | fwereade, hmm. WaitForExpiry | 11:33 |
| frobware | fwereade, though I can do the last as well | 11:33 |
| frobware | fwereade, the For makes it sounds more like present tense | 11:34 |
| dimitern | that works just as well for me :) | 11:34 |
| voidspace | dimitern: WaitForExpiry | 11:34 |
| frobware | voidspace, was your "you around?" for the standup or something else? | 11:41 |
| voidspace | frobware: yeah, just for standup | 12:10 |
| voidspace | dooferlad appears not to be here | 12:10 |
| === Makyo is now known as Guest61971 | ||
| frobware | dimitern, do you know if a runcmd (for cloud init & from juju) always needs to be a shell script? | 12:25 |
| voidspace | frobware: I think cloud-init runs a shell script | 12:26 |
| voidspace | not entirely sure though :-) | 12:26 |
| dimitern | frobware, it needs to be executable AFAIK | 12:27 |
| dimitern | frobware, have a look at https://cloudinit.readthedocs.org/en/latest/topics/examples.html#run-commands-on-first-boot | 12:28 |
| frobware | thx | 12:28 |
| dimitern | frobware, but what I *think* you're asking is why adding a script generates a shell script wrapper? :) | 12:29 |
| frobware | dimitern, actually not sure what I was asking... just wondering how I know get my python-only /e/n/i renderer to work/be added. | 12:30 |
| frobware | dimitern, it's not quite 100% python but I was wondering if I still needed some sh wrapper | 12:30 |
| dimitern | frobware, using AddScripts() creates a wrapper for you | 12:31 |
| dimitern | frobware, it will render something like `#!/bin/bash\nscript1\nscript2...` | 12:31 |
| dimitern | frobware, have a look around the tests in cloudconfig | 12:32 |
| mup | Bug #1520571 opened: Juju destroy-environment stacktraces on local provider. <landscapee> <juju-core:New> <https://launchpad.net/bugs/1520571> | 13:24 |
| mup | Bug #1520571 changed: Juju destroy-environment stacktraces on local provider. <landscapee> <juju-core:New> <https://launchpad.net/bugs/1520571> | 13:27 |
| mup | Bug #1520571 opened: Juju destroy-environment stacktraces on local provider. <landscapee> <juju-core:New> <https://launchpad.net/bugs/1520571> | 13:30 |
| === akhavr1 is now known as akhavr | ||
| dimitern | fwereade, voidspace, dooferlad, frobware, please have a look - http://reviews.vapour.ws/r/3223/ I think the bindings stuff should be good to land | 14:18 |
| dimitern | especially fwereade :) | 14:20 |
| dimitern | voidspace, btw your branch looks like it needs rebasing before trying to land it | 14:21 |
| fwereade | dimitern, ack | 14:23 |
| dimitern | fwereade, cheers :) | 14:24 |
| fwereade | dimitern, reopened one issue, looking at diff now | 14:30 |
| dimitern | fwereade, I thought it was safer to do asserts like that | 14:32 |
| dimitern | fwereade, but I'm happy to drop it if you think it's better | 14:32 |
| fwereade | dimitern, I think it's mildly harmful -- when, e.g. two components are fighting over the doc, I'd much rather one just won the race, rather than risk them getting into edit wars with each other | 14:35 |
| fwereade | dimitern, and also a txn is many times more expensive than a read, and doesn't actually give you any better certainty, someone could always overwrite just after your final confirmatory read | 14:36 |
| dimitern | fwereade, right, sounds sensible | 14:36 |
| fwereade | dimitern, cool | 14:36 |
| fwereade | dimitern, reviewed | 14:57 |
| dimitern | fwereade, tyvm! | 14:59 |
| dooferlad | voidspace: could you take another look at the gomaasapi branch? https://code.launchpad.net/~dooferlad/gomaasapi/subnets/+merge/278342 don't know if you got an update about my update :-) | 15:47 |
| mup | Bug #1520623 opened: juju/charm: Meta can use a CombinedCharmRelations() method <charm> <tech-debt> <juju-core:Triaged> <https://launchpad.net/bugs/1520623> | 15:48 |
| voidspace | dooferlad: yep | 15:50 |
| mup | Bug #1520623 changed: juju/charm: Meta can use a CombinedCharmRelations() method <charm> <tech-debt> <juju-core:Triaged> <https://launchpad.net/bugs/1520623> | 15:51 |
| voidspace | dimitern: ok, I'll rebase it before landing - it needs dooferlad's branch anyway | 15:52 |
| mup | Bug #1520623 opened: juju/charm: Meta can use a CombinedCharmRelations() method <charm> <tech-debt> <juju-core:Triaged> <https://launchpad.net/bugs/1520623> | 15:54 |
| voidspace | dooferlad: how did you address the nil slice -> null problem? It's not obvious to me from the updated diff | 15:58 |
| dimitern | fwereade, ping | 15:59 |
| fwereade | dimitern, pong | 16:00 |
| voidspace | dooferlad: ah, checking the actual diff for that commit helps... | 16:01 |
| dimitern | fwereade, I fixed all but 2 issues, as I have questions there - can you have a final look and LGTM it if you think it can land? | 16:01 |
| voidspace | dooferlad: cool, LGTM | 16:02 |
| dooferlad | voidspace: great. Is there any automatic pre-merge testing for gomaasapi or should I manually merge and push? | 16:06 |
| fwereade | dimitern, heh, honestly I do really dislike that test | 16:08 |
| dimitern | fwereade, which one? | 16:08 |
| fwereade | dimitern, will there be an UpdateEndpointBindings method inn the nearish future? | 16:08 |
| fwereade | dimitern, the one that uses ServcieGlobalKey, and hits the database | 16:08 |
| fwereade | directly | 16:08 |
| voidspace | dooferlad: manual merge I'm afraid | 16:08 |
| dooferlad | voidspace: thats OK by me - just didn't want to skip a step | 16:09 |
| dimitern | fwereade, well, as along as there's NO way to reach it from the CLI | 16:09 |
| voidspace | dooferlad: cool | 16:09 |
| voidspace | dooferlad: nice work | 16:09 |
| dooferlad | voidspace: thanks! | 16:09 |
| dimitern | fwereade, otherwise the whole point of using immutable bindings vs. constraints seems moot | 16:09 |
| fwereade | dimitern, sure -- but this is the model interface we're designing here, right? | 16:10 |
| fwereade | dimitern, that said | 16:10 |
| dimitern | fwereade, yeah | 16:10 |
| fwereade | dimitern, if they're immutable | 16:10 |
| fwereade | dimitern, I'd be comfortable just dropping the what-if-they-change functionality | 16:11 |
| dimitern | fwereade, well, immutable is not the right term - you can add to them but not remove existing ones, except when the charm is upgraded | 16:11 |
| fwereade | dimitern, so you can change from "default" but not from anything else? | 16:12 |
| fwereade | dimitern, that... does not sound right? | 16:12 |
| dimitern | fwereade, ok, I'll add an update method as part of the follow-up which creates relation docs with endpoints having a space name | 16:12 |
| fwereade | dimitern, if we can change from default, surely we can change from anything anyway? | 16:12 |
| dooferlad | voidspace: pushed | 16:13 |
| dimitern | fwereade, the idea is to disallow a scenario where you deploy a service with given bindings, then add some units, change the bindings (same charm) and add some more units | 16:13 |
| voidspace | dooferlad: great | 16:13 |
| fwereade | dimitern, ok -- then how can a change *from* default be any less invalid than any other change? | 16:14 |
| dimitern | fwereade, say you changed the charm and now want to upgrade to it and bind an endpoint differently (e.g. from default to apps) | 16:15 |
| dimitern | fwereade, "default" is not special (except the assumption it exists and is always available as a fallback) | 16:16 |
| fwereade | dimitern, doesn't *any* change open the door to the full set of edge cases? | 16:17 |
| dimitern | fwereade, similar scenario: deployed mysql --bind shared-db@internal, added a relation between mysql and wordpress; now you want to relate it to phpmyadmin, but use a different space: add-relation mysql:shared-db@admin phpmyadmin:db@dmz | 16:18 |
| fwereade | dimitern, right, I think I do understand the use cases | 16:19 |
| dimitern | fwereade, I think any change at the wrong moment will be problematic, but changes at well defined moments (when the relations are created and effectively bound at that point; or when endpoints have changed) | 16:19 |
| dimitern | should be safe IMO | 16:20 |
| fwereade | dimitern, what I'm saying is that doing that correctly is no more or less hard than any other case in which a service's spaces requirements can change with units in the field | 16:20 |
| dimitern | fwereade, agreed - just having updateBindings method won't violate the "immutability" | 16:21 |
| dimitern | fwereade, but I need to grok a few other interactions first and use cases to be able to hold it in my head consistently (and also I need some rest badly I think..) | 16:22 |
| fwereade | dimitern, ok, so long as there's an imminent followup that addresses the db-poking (put it on the tech-debt board), it's not the end of the world | 16:23 |
| dimitern | fwereade, sure, I promise that will be in the very next PR I put up | 16:24 |
| dimitern | fwereade, good idea about the tech-dept board, adding a card now | 16:24 |
| fwereade | dimitern, I am a bit worried that we may be assuming more immutability than actually exists but I also assume that handling for changes will coming along with the refcounting | 16:24 |
| fwereade | dimitern, ok, SGTM | 16:24 |
| fwereade | dimitern, shipited | 16:25 |
| dimitern | fwereade, cheers, and thanks for bearing with me :) | 16:25 |
| fwereade | dimitern, no worries :) | 16:26 |
| voidspace | frobware: ping | 17:54 |
| voidspace | frobware: I can't rebase my branch with maas-spaces | 18:04 |
| voidspace | frobware: not sure why, probably because I merged rather than rebasing at an earlier point | 18:04 |
| voidspace | frobware: but the rebase has horrible, horrible conflicts at every step | 18:04 |
| voidspace | frobware: ok for me to land it as it is? https://github.com/juju/juju/pull/3834 | 18:05 |
| === psivaa is now known as psivaa-afk | ||
| voidspace | ericsnow: ping | 18:21 |
| ericsnow | voidspace: pong | 18:21 |
| voidspace | ericsnow: your PR 3243 | 18:21 |
| voidspace | Wrap state functionality for resource specs and register the component. | 18:21 |
| ericsnow | voidspace: yep | 18:21 |
| voidspace | ericsnow: resource/state/state.go is in the state package | 18:21 |
| voidspace | ericsnow: is that the same state package as the main state package? | 18:21 |
| ericsnow | voidspace: no | 18:22 |
| voidspace | ericsnow: ah, subpackage? | 18:22 |
| ericsnow | voidspace: subpackage of the new "resource" package | 18:22 |
| voidspace | ericsnow: it seems to me that having another state.State is a recipe for confusion | 18:22 |
| voidspace | but that may just be because I'm confused already :-) | 18:23 |
| ericsnow | voidspace: heh | 18:23 |
| * voidspace goes to read the "packages in go" document | 18:23 | |
| ericsnow | voidspace: the subpackages of the "resource" package are essentially internal to the new feature | 18:24 |
| ericsnow | voidspace: I'm fine with renaming State to something more obvious if you think that will help alleviate any confusion | 18:25 |
| mup | Bug #1520669 opened: Upgrade from 1.21.3 -> 1.25.0 got minor errors <juju-core:New> <https://launchpad.net/bugs/1520669> | 18:45 |
| mup | Bug #1520669 changed: Upgrade from 1.21.3 -> 1.25.0 got minor errors <juju-core:New> <https://launchpad.net/bugs/1520669> | 18:48 |
| mup | Bug #1520669 opened: Upgrade from 1.21.3 -> 1.25.0 got minor errors <juju-core:New> <https://launchpad.net/bugs/1520669> | 18:54 |
| natefinch | gah, sometimes I just hate git | 20:09 |
| natefinch | today is one of those days | 20:09 |
| natefinch | ericsnow: for the feature branches we've worked on, did you rebase them onto master periodically, or was that a merge? | 20:16 |
| ericsnow | merge | 20:16 |
| natefinch | ericsnow: ok, yeah, realized after spending a bunch of time rebasing my feature branch that I can't really make a PR with that against the current one :) | 20:17 |
| ericsnow | natefinch: yep :) | 20:18 |
| natefinch | anyone online have write access to the juju/charm repo? I need a feature branch created | 20:40 |
| natefinch | cmars, abentley: can one of you make a branch off juju/charm v6-unstable called nate-minver? I need a feature branch I can check code into so my feature branch in juju/juju can build, and I can't create the branch myself. | 20:45 |
| abentley | natefinch: looking... | 20:59 |
| abentley | natefinch: should be there now. | 21:01 |
| natefinch | abentley: epic. Thanks! | 21:01 |
| abentley | natefinch: No worries. | 21:01 |
| natefinch | abentley: will that automatically get picked up by the merge bot, or is there more than needs to be done for that? | 21:03 |
| abentley | natefinch: I don't know about merging for this repo, only juju/juju. mgz will know, but he's EOD. | 21:05 |
| natefinch | abentley: kk | 21:08 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!