davechen1ythumper: menn0 axw https://github.com/juju/testing/pull/8500:15
davechen1y^ for skipping go 1.5 failing tests00:15
mupBug #1520380 opened: worker/provisioner: unit tests fail on xenial <juju-core:New> <https://launchpad.net/bugs/1520380>00:19
thumperdavechen1y: kk00:21
menn0thumper: ship it00:21
thumperdavechen1y: why use the testing change instead of just using the go version in the code?00:22
thumperdavechen1y: what does it buy us?00:22
davechen1ythumper: same as https://github.com/juju/testing/pull/8400:23
davechen1ywe're probably going to need to do it in a bunch of places00:24
davechen1yusing build constraints requires at least 5 files00:24
davechen1ybetter to put it in one place, than replicate those turds across every package we have to skip conditionally00:24
thumpergood enough for me00:24
davechen1yalso, when we're done with this, it's one place to delete00:25
* thumper nods00:25
menn0davechen1y: 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 out00:25
davechen1ymenn0: yes, and no00:25
menn0Parsing the output of https://golang.org/pkg/runtime/#Version is another option00:25
davechen1yyou only need to add a new file if you want to identify that version of go00:25
davechen1ygo 1.6 will identify as go 1.5 under this scheme00:25
davechen1yand also, when/if that happens, it happens in one place00:25
davechen1yand when we're done with this aweful hack, we can delete it in one palce00:26
davechen1yand when we're done with this aweful hack, we can delete it in one place00:26
menn0davechen1y: 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
davechen1yit would be messier00:27
davechen1yparsing, yeah easy enough00:27
davechen1ybut we'd lose the constant nature of these constnts00:27
davechen1yyou'd get a float64, and get shat on by precision00:27
davechen1yequals may not work00:27
davechen1ydoing it this way gives us equality between ideal constants00:28
davechen1yif testing.GOVERSION == 1.5 is a constant expression00:28
davechen1yi suspect if we did it by parsing runtime.Version, we'd end up _also_ defining a bunch of constants then returning those constnat values00:29
davechen1ywhich would also be forced throught float64 coercion, blah blah00:29
davechen1ythis is less worse doing it this way00:30
davechen1yand if it feels a bit icky, that's good00:30
davechen1ywe should be skiping all this crap00:30
davechen1y^ trivial review00:34
davechen1yaxw: mgz_ http://juju-ci.vapour.ws/job/run-unit-tests-xenial-amd64/129/consoleFull00: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 happens00:35
mgz_from last night though, if you want to re-run a job00:36
mgz_just login as developer per creds in lp:cloud-city00: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
davechen1yok, so if I land a fix for the xenial test failure, i should wait for build-revision to pick it up00:37
mgz_yeah, to run a new revison just wait for it to go through ci as normal00:37
davechen1ymgz_: I noticed that if I kill a -race job that is working on a branch which I know will never pass00:39
davechen1yci just runs it again00:39
davechen1yis there any way to avoid this ?00:39
davechen1yit wastes 3 hours every time it chews on some branch that will never pass00:39
mgz_davechen1y: change failure-threshhold to 100:40
mgz_issue with that being... the tests do randomly fail sometimes00:40
davechen1ymgz_: meh00:40
mgz_so we get curses which retesting will 'cure;00:40
davechen1yif they fail sometimes, i'll comment 'em out00:41
mgz_davechen1y: what you can do for that manual thing00:41
mgz_is change it to one then... hm.. actually, you'd need to change it back after the entire run finished, which would suck00:41
mgz_davechen1y: the problem with the just0skip-it approach00:42
davechen1yit sounds like I should just live with it00:42
davechen1ythis problem will solve itself soon enough when feature branches rebase00:42
mgz_well, problem one is we've been historically bad at fixing unreliable tests, skipping them just makes the problem easier to ignore00:42
davechen1ymgz_: no argument there00: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 now00:43
mgz_anyway, problem two with skipping tests is sometimes it's like, "this entire suite is flakey"00:43
davechen1ymgz_: nah, this problem will solve itself soon enough00:43
mgz_losing *all* our uniter coverage say, is bad00:43
davechen1ymgz_: no arguemnt there00:44
mgz_we've shipped bad code in the past because tests have been ignored00:44
davechen1yi'm stuck between a rock and a hard place00:44
davechen1ythumper: do you have any comment on this position ?00:44
thumperhow far back do I need to read?00:44
davechen1y2 minutes00:45
davechen1y4 at most00:45
davechen1ymgz_: rightly points out that historically skipping a failing package takes the issue off the radar00:45
mgz_thumper: I write concisely and with clarity, surely reading isn't that much of a chore for you ;_;00:45
davechen1yi don't disagree, but historically fixing flakey tests, especially races as been an uphill battle00:45
davechen1ymeaning we have no race coverage, so nothing prevening new races coming in00:45
mgz_I think your approach on making -race useful was good00:46
thumperI understand the races one00:46
thumperas people kept adding them too frequently00:46
mgz_the tests just took way too long and had too much random output to be good without culling a bunch00:46
thumperI'm guessing that the wily failures are more limited?00:46
thumperas in we know which ones fail often?00:46
davechen1yxenial has 1 failure00:47
mgz_welll... there's a few obvious bugs, like lxd borked a few tests00:47
davechen1yskipping that for go 1.5 gets us a voting go 1.5 build00:47
mgz_and there's a few new intermittent failures, which... could be a number of things00:47
davechen1ywhich is crucial for upgrading to go 1.500:47
thumperwhat's the difference between failures on xenial and wily?00:47
davechen1yno idea00:47
davechen1yhaven't looked at wily00:47
mgz_and we get, possibily just a different symptom of, mongodb related failure00:47
mgz_they're much the same00:47
mgz_the package set hasn't really diverged, and it's all go 1.500:48
* davechen1y is not touching the mongodb debt clusterfuck00:48
mgz_if it's just the mongodb panic we can probably contain that00:48
mgz_run three times, hope, etc, as previously00:48
thumperdavechen1y: how about just fixing the test rather than skipping it if it is just one?00:49
thumpermy main concern is that with 1.500:49
thumperif we are skipping a lot of tests00:50
thumperand losing coverage00:50
thumperwe are losing faith that the code works00:50
davechen1ythumper: this is worker/provisioner00:50
davechen1yits riddled with timing issues00:50
davechen1ythere have been bugs open against that packge for near 2 years00:50
davechen1yso clearly there is no priority in fixing it00:51
thumperso my point stands, if go 1.5 is more likely to cause problems in the provisioner due to the way things run00:52
thumperI'd rather have a failing test than a skipped test00:52
thumpermy stance is generally this:00:53
thumperskipping shitty tests is one thing00:53
thumperbut skipping good tests that show bad code is wrong00: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 names00:53
davechen1yyou're fine with skipping packages with known data races, but you're concerned about skipping a package with logical timing issues ?!?00:53
davechen1ymgz_: thanks!00:53
thumperdavechen1y: well...00:54
thumperthe tests pass.. FSVO pass00:54
davechen1yyou cannot choose which is the lesser issue00:54
thumperperhaps it is just a false sense of security00:54
davechen1yyou cannot choose which is the lesser evil00:54
davechen1yevery package I skip is tracked with an LP issue00:54
davechen1ymake them critical blockers if you like00:54
thumperdavechen1y: do you have tags on them?00:55
thumperif not, please add some00: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 blue00:55
davechen1ythumper: nope, no tags00:55
davechen1ywhich tags would you like ?00:55
mgz_is what I have used00:56
davechen1yno new tag00:56
thumperhow about skipped-test + ci-failure + ci-test name?00:56
davechen1ywe have so many tags for "critical", "omg critical", "exploding customer", etc00:56
davechen1yadding another tag is a net negative00:56
davechen1yI think we should use milestones and LP status00:56
thumpera tag for skipped-test is fine00:56
davechen1yi disagree00:58
davechen1yhistorically tags have proved bad at communicating severity or priority00:58
davechen1yi think we should use LP severity00:58
mgz_I agree with that, but it's not really a tag thing00:58
davechen1ywhich will automatically fit into the work that cherylj is doing with her dashboards00:58
davechen1yfor example, we already have several tags for data-race00: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
davechen1ynone have proven to motivate people to fix them00:59
davechen1ymgz_: no argument here00:59
davechen1ymgz_: no argument there00: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-mode01:00
davechen1ymgz_: yup, so here is my proposition01:01
davechen1yskip the tests, get the jobs voting, fix the skipped tests01:01
davechen1ywe've tried to do it in another order for some time now01:01
davechen1ywithout success01:01
davechen1ytime to try something else01:01
davechen1ynote: tests are skiped only in specific conditions, which are only true for a subset of CI builds01:01
davechen1yI am not unilaterally disabling the tests of a packge01:02
davechen1ymgz_: thumper for example, https://github.com/juju/juju/pull/3840/files01:02
mupBug #1520380: worker/provisioner: unit tests fail on xenial <juju-core:Confirmed> <https://launchpad.net/bugs/1520380>01:05
axwdavechen1y: I think I have a fix for that test01:24
thumperfound another bug01:25
thumperwhen status should indicate that an agent is down01:26
thumperit only sets the old status01:26
thumperah this is so f'ed up01:28
davechen1ythumper: where was the log and return nil ?01:31
thumperinstance poller01:32
thumperit should be setting the instance status to missing / unknown / down whatever01:32
davechen1ythumper: urgh01:33
davechen1ydo you want me to take a pass at fixing that, or are you on it ?01:33
davechen1yaxw: sure, if you reckon you have a fix then we can put my PR on the shelf01:34
axwdavechen1y: just running all tests now, should have a PR up shortly01:34
davechen1yaxw: thanks!01:38
thumperwallyworld: what is up with your hangouts today?01:44
wallyworldi'm here01:44
thumperI can't hear you on the hangout01:45
wallyworldthumper: reconnecting, google has been bad today for me01:45
wallyworlddocs as well01:45
axwdavechen1y: FYI, http://reviews.vapour.ws/r/3260/01:48
davechen1yaxw: hmm01:54
davechen1ythat isn't the same issue01:54
mupBug #1520380: worker/provisioner: unit tests fail on xenial <juju-core:Confirmed> <https://launchpad.net/bugs/1520380>01:54
thumperwallyworld: sorry, giving up on the hangout01:55
wallyworldthumper: yeah, sorry01:55
wallyworldgoogle is shite today01:56
thumperwallyworld: I'm going to write up a small spec re presense properly01:56
axwdavechen1y: it is, grep for "unknown container type: lxd"01:56
davechen1yfair enough01:56
davechen1ylets commit it and see what happens01:56
thumperaxw: do you have a few minutes? wallyworld said you have thought quite a bit about presense already01:58
thumperand I'm right in that head space01:58
thumperaxw: no worries if you don't have time01:58
axwthumper: can do, but meant to have a 1:1 with wallyworld in 2mins01:58
wallyworldaxw: we can delay our 1:101:58
wallyworldping me when free01:58
axwthumper: sure, name a place01:58
axwwallyworld: okey dokey01:58
thumperaxw: https://plus.google.com/hangouts/_/canonical.com/presence?authuser=101:59
axwwallyworld: ready02:14
mgz_menn0: I <3 u02:16
davechen1ythumper: mwhudson https://groups.google.com/d/topic/golang-dev/y-mlM-XYysk/discussion02:16
wallyworldmgz_: do you want to share a room with him?02:16
mgz_that may now be dangerous02:17
* menn0 is glad we have our own rooms in Oakland02:17
axwdavechen1y: nice :)02:17
mwhudsondavechen1y: oh hooray02:17
davechen1yi'm gobsmacked they did it in secrecy02:18
mwhudsonyeah, seems insane02:18
mwhudsonglad they're talking about it02:18
mwhudsonstill no link to the source though...02:18
menn0mgz_: 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
davechen1ymwhudson: i do hope they based their work of Go 1.5 or later02:19
mgz_yeah, we should have apparently just kept the whol odd-numbers-are-bad thing02:19
davechen1yif they used 1.402:19
mwhudsondavechen1y: ah uh yes02:19
davechen1ythat will be, complicated02:19
anastasiamacmgz_: menn0 <3s scotch (probably safer than sharing a room...)02:19
menn0mgz_: and before you get too excited... the bug I found was created by me :(02:21
* menn0 hangs his head in shame02:21
mgz_ehehe, that just makes it more amusing02:21
anastasiamacdouble the scotch then \o/02:21
menn0i'm glad it only exists in 1.21.N releases02:22
thumperdavechen1y: nice02:30
mupBug #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
davechen1yaxw: sorry, your change did not fix the problem I am seeing02:52
davechen1ymgz_: the build-revision job appears to be disabled02:53
davechen1yis that how it works, or is that a mistake ?02:53
davechen1yaxw: http://paste.ubuntu.com/13522160/02:53
axwdavechen1y: is that from juju-ci, or your machine?02:59
davechen1ymy machine02:59
davechen1ybut matches exaclty what I see for the xenial build02:59
axwdavechen1y: gah, I must have missed something. I'm getting it too03:00
axwlooking now03:01
axwdavechen1y: no, sorry, I just hadn't pulled master. works on my machine ... :/03:03
axwdavechen1y: can you confirm that "instance/container_go13.go" is not in your tree?03:03
davechen1ylucky(~/src/github.com/juju/juju/worker/provisioner) % ls03:04
davechen1ycontainer_initialisation.go       export_test.go  kvm-broker_test.go  logging_test.go  lxc-broker_test.go  provisioner.go       provisioner_test.go03:04
davechen1ycontainer_initialisation_test.go  kvm-broker.go   logging.go          lxc-broker.go    package_test.go     provisioner_task.go03:04
davechen1yhmm, are we looking at the same thing ?03:05
davechen1yafk for a bit03:05
axwdavechen1y: what's in github.com/juju/juju/instance ?03:05
davechen1yok, crisis averted03:18
davechen1yaxw: lucky(~/src/github.com/juju/juju/instance) % ls03:29
davechen1ycontainer.go  container_test.go  instance.go  instance_test.go  placement.go  placement_test.go  testing03:29
axwdavechen1y: welp, I dunno then. I kicked off a build-revision job anyway03:29
thumperwallyworld: I have a few questions about unit agent status03:57
axwdavechen1y: http://juju-ci.vapour.ws:8080/job/run-unit-tests-xenial-amd64/133/console04:01
axwdavechen1y: Finished: SUCCESS04:01
davechen1yweird, it still fails 100% for me04:02
axwdavechen1y: only thing I can think of is that the juju/instance package didn't rebuild04:03
davechen1yaxw: i have a change coming04:07
davechen1yits not a logic error04:08
thumperlaters folks04:24
menn0wallyworld or axw: http://reviews.vapour.ws/r/3262/ please04:50
menn0wallyworld or axw: no major rush as I'm done for the week04:50
menn0wallyworld: thanks04:51
wallyworldmenn0: first glance, looks like a lot of moving of code?04:51
menn0wallyworld: yes, and a lot of changes to the moved code04:51
menn0wallyworld: the PR and separate commits give more detail04:52
wallyworldok, will look a little later04:52
menn0wallyworld: maybe it's easier to look at the individual commits on Github?04:52
=== akhavr1 is now known as akhavr
voidspacedimitern: ping - if you have a minute09:03
dimiternvoidspace, sure09:04
voidspacedimitern: so maas does support ipv6 subnets (max of one per interface)09:04
dimiternvoidspace, I didn't know it's just one09:04
voidspacedimitern: my PR is done, most of your suggestions are in (tests refactoring and reformatting etc)09:04
voidspacedimitern: according to their ipv6 docs09:04
dimiternvoidspace, cheers09:04
dimiternvoidspace, I'll have another look before standup09:05
voidspacedimitern: however my code as written won't work with ipv6 - it assumes ipv4 for working out the allocatable range09:05
voidspacedimitern: there's a TODO in there as you suggested09:05
voidspacedimitern: should I fix it (without manual tests as I don't have an ipv6 setup)09:05
voidspacedimitern: or ok to leave it as a TODO for now?09:05
dimiternvoidspace, as long as we keep it in mind, a TODO is fine for now I think09:06
voidspacedimitern: cool09:06
voidspacedimitern: I'll let you have another look before I land it then09:06
voidspacedimitern: the only thing I didn't do was the extra work in the dummy provider09:06
voidspacedimitern: I figure we can do that when we actually need it09:06
voidspaceit would be unused code until then09:06
voidspaceand I have ethical objections to unused code09:07
dimiternvoidspace, :) agreed - we can add it later, leaving a TODO there with some of the things we need would be good though - I'll comment09:08
dimiternvoidspace, reviewed09:33
voidspacedimitern: subnetIdSet is a map, so ranging over it gets the keys and values not an index and value09:35
voidspacedimitern: and it is a map because the bool value has semantic meaning09:35
voidspacedimitern: if you read the code notFound is *not* the same length as subnetIdSet09:35
voidspacedimitern: we only put in it the not found ones09:36
dooferladdimitern, voidspace: why are we treating IPv4 and IPv6 addresses differently at all? They can both be represented as uint64.09:36
dooferladdimitern, voidspace: subnet masks the same09:36
voidspacedimitern: that's true for both your first two comments09:36
dimiternvoidspace, ah, I was too quick then :) I'll drop that issue09:37
dimiternvoidspace, we should use sets when possible though09:38
dimiternin cases like this09:38
voidspacedooferlad: because the code as written only works with IPv4 and would require rewriting09:38
voidspacedimitern: did you read what I wrote...09:38
voidspacedimitern: about why it is a map09:38
dimiternvoidspace, still, the second comment about using a helper for extracting notFound from subnetIdsSet holds09:38
dimiternvoidspace, yeah09:39
voidspacedimitern: it needs to be a map09:39
dimiternvoidspace, yeah09:39
voidspacedimitern: ok, I can do that09:39
voidspacedooferlad: plus it would need some code to track if it's ipv4 or ipv6 addresses we're using09:41
voidspacedooferlad: as addressing need converting back to the correct format09:41
voidspacedooferlad: it's not hard it's just not zero work09:42
dooferladvoidspace: I mostly bring it up because it isn't hard and by landing this we introduce tech debt.09:44
dooferladvoidspace: that said, I am happy to write a followup.09:45
dooferladvoidspace: (as I am sure you would be)09:45
voidspacedooferlad: cool09:45
voidspacedooferlad: yep, there's a couple of places that use that conversion code that would need changing09:47
voidspacedooferlad: definitely worth doing09:47
voidspacedimitern: that trailing bracket was an oversight :-p09:47
voidspacedimitern: thanks for catching it09:47
dooferladvoidspace: thankfully IPv4ToDecimal is used in four places before your change (other than tests), so killing it with fire is easy09:47
dimiternvoidspace, any time :)09:48
voidspacedimitern: changes pushed09:51
voidspacedimitern: it actually needs dooferlad's changes to gomaasapi to land first09:51
voidspacedooferlad: did you get a review for your branch?09:51
voidspacedooferlad: if not I'll look at it09:51
dooferladvoidspace: no, needs a review09:51
dimiternvoidspace, cheers - let's land it09:51
voidspacedooferlad: 1400 lines!09:54
voidspacedooferlad: I'm concerned that the struct approach to serialising sends empty (uninitialised) arrays as null09:55
voidspacedooferlad: which doesn't match maas and will break code that does GetArray()09:55
dooferladvoidspace: happy to run through it with you after the hangout09:59
voidspacefrobware: you around?10:02
dimiternfwereade, jam, standup?10:02
frobwaredimitern, strike 310:04
voidspacedimitern: jam is never around on Friday :-)10:04
voidspacedooferlad: will sync up with you shortly10:32
dooferladvoidspace: cool10:32
=== beisner- is now known as beisner
=== cppforlife__ is now known as cppforlife_
fwereadestraw poll: WaitExpired, WaitForExpiry, WaitUntilExpired?11:31
fwereademgz, dimitern, frobware, voidspace? ^11:31
dimiternfwereade, last one11:32
fwereadedimitern, cheers11:33
frobwarefwereade, hmm. WaitForExpiry11:33
frobwarefwereade, though I can do the last as well11:33
frobwarefwereade, the For makes it sounds more like present tense11:34
dimiternthat works just as well for me :)11:34
voidspacedimitern: WaitForExpiry11:34
frobwarevoidspace, was your "you around?" for the standup or something else?11:41
voidspacefrobware: yeah, just for standup12:10
voidspacedooferlad appears not to be here12:10
=== Makyo is now known as Guest61971
frobwaredimitern, do you know if a runcmd (for cloud init & from juju) always needs to be a shell script?12:25
voidspacefrobware: I think cloud-init runs a shell script12:26
voidspacenot entirely sure though :-)12:26
dimiternfrobware, it needs to be executable AFAIK12:27
dimiternfrobware, have a look at https://cloudinit.readthedocs.org/en/latest/topics/examples.html#run-commands-on-first-boot12:28
dimiternfrobware, but what I *think* you're asking is why adding a script generates a shell script wrapper? :)12:29
frobwaredimitern, 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
frobwaredimitern, it's not quite 100% python but I was wondering if I still needed some sh wrapper12:30
dimiternfrobware, using AddScripts() creates a wrapper for you12:31
dimiternfrobware, it will render something like `#!/bin/bash\nscript1\nscript2...`12:31
dimiternfrobware, have a look around the tests in cloudconfig12:32
mupBug #1520571 opened: Juju destroy-environment stacktraces on local provider. <landscapee> <juju-core:New> <https://launchpad.net/bugs/1520571>13:24
mupBug #1520571 changed: Juju destroy-environment stacktraces on local provider. <landscapee> <juju-core:New> <https://launchpad.net/bugs/1520571>13:27
mupBug #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
dimiternfwereade, voidspace, dooferlad, frobware, please have a look - http://reviews.vapour.ws/r/3223/ I think the bindings stuff should be good to land14:18
dimiternespecially fwereade :)14:20
dimiternvoidspace, btw your branch looks like it needs rebasing before trying to land it14:21
fwereadedimitern, ack14:23
dimiternfwereade, cheers :)14:24
fwereadedimitern, reopened one issue, looking at diff now14:30
dimiternfwereade, I thought it was safer to do asserts like that14:32
dimiternfwereade, but I'm happy to drop it if you think it's better14:32
fwereadedimitern, 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 other14:35
fwereadedimitern, 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 read14:36
dimiternfwereade, right, sounds sensible14:36
fwereadedimitern, cool14:36
fwereadedimitern, reviewed14:57
dimiternfwereade, tyvm!14:59
dooferladvoidspace: 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
mupBug #1520623 opened: juju/charm: Meta can use a CombinedCharmRelations() method <charm> <tech-debt> <juju-core:Triaged> <https://launchpad.net/bugs/1520623>15:48
voidspacedooferlad: yep15:50
mupBug #1520623 changed: juju/charm: Meta can use a CombinedCharmRelations() method <charm> <tech-debt> <juju-core:Triaged> <https://launchpad.net/bugs/1520623>15:51
voidspacedimitern: ok, I'll rebase it before landing - it needs dooferlad's branch anyway15:52
mupBug #1520623 opened: juju/charm: Meta can use a CombinedCharmRelations() method <charm> <tech-debt> <juju-core:Triaged> <https://launchpad.net/bugs/1520623>15:54
voidspacedooferlad: how did you address the nil slice -> null problem? It's not obvious to me from the updated diff15:58
dimiternfwereade, ping15:59
fwereadedimitern, pong16:00
voidspacedooferlad: ah, checking the actual diff for that commit helps...16:01
dimiternfwereade, 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
voidspacedooferlad: cool, LGTM16:02
dooferladvoidspace: great. Is there any automatic pre-merge testing for gomaasapi or should I manually merge and push?16:06
fwereadedimitern, heh, honestly I do really dislike that test16:08
dimiternfwereade, which one?16:08
fwereadedimitern, will there be an UpdateEndpointBindings method inn the nearish future?16:08
fwereadedimitern, the one that uses ServcieGlobalKey, and hits the database16:08
voidspacedooferlad: manual merge I'm afraid16:08
dooferladvoidspace: thats OK by me - just didn't want to skip a step16:09
dimiternfwereade, well, as along as there's NO way to reach it from the CLI16:09
voidspacedooferlad: cool16:09
voidspacedooferlad: nice work16:09
dooferladvoidspace: thanks!16:09
dimiternfwereade, otherwise the whole point of using immutable bindings vs. constraints seems moot16:09
fwereadedimitern, sure -- but this is the model interface we're designing here, right?16:10
fwereadedimitern, that said16:10
dimiternfwereade, yeah16:10
fwereadedimitern, if they're immutable16:10
fwereadedimitern, I'd be comfortable just dropping the what-if-they-change functionality16:11
dimiternfwereade, well, immutable is not the right term - you can add to them but not remove existing ones, except when the charm is upgraded16:11
fwereadedimitern, so you can change from "default" but not from anything else?16:12
fwereadedimitern, that... does not sound right?16:12
dimiternfwereade, ok, I'll add an update method as part of the follow-up which creates relation docs with endpoints having a space name16:12
fwereadedimitern, if we can change from default, surely we can change from anything anyway?16:12
dooferladvoidspace: pushed16:13
dimiternfwereade, 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 units16:13
voidspacedooferlad: great16:13
fwereadedimitern, ok -- then how can a change *from* default be any less invalid than any other change?16:14
dimiternfwereade, 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
dimiternfwereade, "default" is not special (except the assumption it exists and is always available as a fallback)16:16
fwereadedimitern, doesn't *any* change open the door to the full set of edge cases?16:17
dimiternfwereade, 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@dmz16:18
fwereadedimitern, right, I think I do understand the use cases16:19
dimiternfwereade, 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
dimiternshould be safe IMO16:20
fwereadedimitern, 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 field16:20
dimiternfwereade, agreed - just having updateBindings method won't violate the "immutability"16:21
dimiternfwereade, 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
fwereadedimitern, 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 world16:23
dimiternfwereade, sure, I promise that will be in the very next PR I put up16:24
dimiternfwereade, good idea about the tech-dept board, adding a card now16:24
fwereadedimitern, 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 refcounting16:24
fwereadedimitern, ok, SGTM16:24
fwereadedimitern, shipited16:25
dimiternfwereade, cheers, and thanks for bearing with me :)16:25
fwereadedimitern, no worries :)16:26
voidspacefrobware: ping17:54
voidspacefrobware: I can't rebase my branch with maas-spaces18:04
voidspacefrobware: not sure why, probably because I merged rather than rebasing at an earlier point18:04
voidspacefrobware: but the rebase has horrible, horrible conflicts at every step18:04
voidspacefrobware: ok for me to land it as it is? https://github.com/juju/juju/pull/383418:05
=== psivaa is now known as psivaa-afk
voidspaceericsnow: ping18:21
ericsnowvoidspace: pong18:21
voidspaceericsnow: your PR 324318:21
voidspaceWrap state functionality for resource specs and register the component.18:21
ericsnowvoidspace: yep18:21
voidspaceericsnow: resource/state/state.go is in the state package18:21
voidspaceericsnow: is that the same state package as the main state package?18:21
ericsnowvoidspace: no18:22
voidspaceericsnow: ah, subpackage?18:22
ericsnowvoidspace: subpackage of the new "resource" package18:22
voidspaceericsnow: it seems to me that having another state.State is a recipe for confusion18:22
voidspacebut that may just be because I'm confused already :-)18:23
ericsnowvoidspace: heh18:23
* voidspace goes to read the "packages in go" document18:23
ericsnowvoidspace: the subpackages of the "resource" package are essentially internal to the new feature18:24
ericsnowvoidspace: I'm fine with renaming State to something more obvious if you think that will help alleviate any confusion18:25
mupBug #1520669 opened: Upgrade from 1.21.3 -> 1.25.0 got minor errors <juju-core:New> <https://launchpad.net/bugs/1520669>18:45
mupBug #1520669 changed: Upgrade from 1.21.3 -> 1.25.0 got minor errors <juju-core:New> <https://launchpad.net/bugs/1520669>18:48
mupBug #1520669 opened: Upgrade from 1.21.3 -> 1.25.0 got minor errors <juju-core:New> <https://launchpad.net/bugs/1520669>18:54
natefinchgah, sometimes I just hate git20:09
natefinchtoday is one of those days20:09
natefinchericsnow: for the feature branches we've worked on, did you rebase them onto master periodically, or was that a merge?20:16
natefinchericsnow: 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
ericsnownatefinch: yep :)20:18
natefinchanyone online have write access to the juju/charm repo?  I need a feature branch created20:40
natefinchcmars, 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
abentleynatefinch: looking...20:59
abentleynatefinch: should be there now.21:01
natefinchabentley: epic.  Thanks!21:01
abentleynatefinch: No worries.21:01
natefinchabentley: will that automatically get picked up by the merge bot, or is there more than needs to be done for that?21:03
abentleynatefinch: I don't know about merging for this repo, only juju/juju.  mgz will know, but he's EOD.21:05
natefinchabentley: kk21:08

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!