/srv/irclogs.ubuntu.com/2014/10/23/#juju-dev.txt

waiganithumper: http://reviews.vapour.ws/r/177/00:27
anastasiamacdavecheney: could u please review http://reviews.vapour.ws/r/235/ - it'd b gr8 to land it :-)01:44
davecheneyanastasiamac: sure01:44
anastasiamacdavecheney: thnx!01:45
davecheneyanastasiamac: not lgtm01:46
davecheneysorry01:46
davecheneyconfig.AgentMetadataURLKey: "01:46
davecheneyi don't think this payus for itself01:46
davecheneyare we really going to be changing the definition of this so frequently that we need to move it's definition to a shared location ?01:46
davecheneythe reason I say this is01:47
davecheneyall the test fixtures still assert it's a constant01:47
anastasiamacdavecheney: u mean using variables instead of string?01:47
davecheneyyup01:47
menn0thumper: https://github.com/mjs/juju/commit/b7f48aa500f6aba285320c6a4aa184736d287b1a01:47
davecheneyanastasiamac: all the fixtures still encode the string directly01:47
davecheneyso does the documentation and the examples01:47
davecheneyi don't feel this pays for itself01:47
menn0thumper: this is the commit that adds various hacks to allow the machine agent to start so that it can run the machine env UUID upgrades01:48
* thumper wonders how horrible it is01:48
menn0thumper: it's actually mostly done, except for some unit test changes01:48
thumperok, it doesn't look TOO long01:48
anastasiamacdavecheney: m not big fan of retyping the same string. if a string is used more thn once, it should b a variable...01:48
menn0thumper: it's a bit horrible but the horribleness is fairly localised01:49
davecheneyanastasiamac: but you ended up typing a slightly longer variable ...01:49
menn0thumper: instead of storing and using a DB version, the code checks for evidence of the upgrade being in place01:50
menn0thumper: this is safer IMHO01:50
menn0thumper: because otherwise you end up with points in time where the DB has been changed but the version hasn't01:51
davecheneythumper: i will be late for the team meeting01:51
davecheneyhave 1:1 with robbiew01:51
thumperkk01:51
davecheneyericsnow: reviews.ws has gone back to reporting it's ec2 url again01:54
davecheneyjust got another email from the wrong address01:54
anastasiamacdavecheney: :-) what length would u consider k for a variable? also should the name be descriptive? eg. here would AgentURL be k?01:56
ericsnowdavecheney: (╯°□°)╯︵ ┻━┻01:56
menn0thumper: does this approach look ok to you?01:57
* thumper looks in detail01:58
thumperanastasiamac: I like descriptive names01:58
thumperplease don't be sucked in to thinking shorter is better01:58
* thumper is opinionated on this01:58
anastasiamacthumper: i agree. shorter is not always practical..01:59
anastasiamacthumper: by the same token, metadata is 8 chars... ;-(01:59
davecheneyanastasiamac: i'm not arguging about the length01:59
davecheneyits the fact that this change doesn't pay for itself02:00
davecheneyyou can't turn all the test fixtures into variables as well02:00
davecheneyor the examples02:00
davecheneyor the documentation02:00
davecheneyor the yaml keys02:00
davecheneyetc02:00
thumpermenn0: mostly looks ok02:02
thumpernot gone through in exceptional detail yet02:02
davecheneyseriously, i missed the meeting ?02:36
thumperdavecheney: yeah...02:49
thumperdavecheney: we were done in 15 minutes02:49
thumperdavecheney: it seems that rb doesn't pick up juju/cmd02:51
thumperhttps://github.com/juju/cmd/pull/802:51
thumperwallyworld_: or you maybe? ^^^02:53
thumperor axw...02:53
wallyworld_wot02:53
thumpersimple change02:53
wallyworld_ok02:53
thumperjuju/cmd update02:53
wallyworld_thumper: are other sub command affected besides juju user?02:56
thumperwallyworld_: it only impacted nested subcommands02:57
thumperand only if you didn't say anything else02:57
wallyworld_ok02:57
thumperso 'juju user' would miss out the 'juju' prefix in the help output02:57
thumperjuju user --help was fine02:57
thumperjuju user help add was fine02:57
thumperjuju user add --help was fine02:57
thumperjuju user was not02:58
wallyworld_ack02:58
wallyworld_sadly you still need to hit the merge button manually for this sub repo i think02:58
thumperwill do02:58
thumperwallyworld_: I also noticed that all the licences were wrong02:59
thumperwallyworld_: I'll submit another quick one to fix that02:59
wallyworld_for cmd/juju?02:59
wallyworld_i thought robbie had audited everything02:59
thumperit still shows AGPL03:00
thumperinstead of LGPL with exception03:00
thumperOH FFS03:03
thumpershould the file be LICENCE or LICENSE?03:03
thumperwe have a mix03:04
thumperLICENSE03:04
thumperaccording to the world03:04
anastasiamacthumper: one is noun, on is verb..03:05
thumperWTF?...03:07
anastasiamacthumper: ~ce (from French/Latin), ~se (from Mid English)...03:07
* thumper found a weird file...03:07
thumperwallyworld_: https://github.com/juju/cmd/pull/903:10
wallyworld_ceis noun03:11
wallyworld_se is verb03:11
wallyworld_like practice and practise03:11
wallyworld_and advise and advice03:11
thumperI always get that shit wrong03:11
thumperwallyworld_: so in that PR, you'll notice I also remove names.go03:11
wallyworld_easy to remember - ice is a noun'03:11
thumperwhich should never have been there in the first place03:11
wallyworld_ok03:12
wallyworld_why are we changing LICEENCE to be incorrectly spelt?03:12
wallyworld_oh the irony. LICENCE03:12
thumperbecause this is what the world has03:13
thumperoh it also appears to be an americanism03:13
wallyworld_yes :-(03:14
wallyworld_americans cannot spell for shit03:14
thumperhttp://oss-watch.ac.uk/resources/opensourceyourcode#applying-the-licence03:14
thumpersays to have LICENSE03:14
wallyworld_we are an englidh company03:14
thumpernot LICENCE03:14
wallyworld_english03:14
thumperthat is a .uk site03:14
wallyworld_but the farking url says licence03:14
thumpernotice the header: Applying The Licence03:15
wallyworld_sigh03:15
thumperGNU LESSER GENERAL PUBLIC LICENSE03:15
thumpernotice the SE03:15
wallyworld_but but but03:15
wallyworld_sigh03:15
thumperdon't shoot me03:15
thumperI'm just trying to be consistent03:15
wallyworld_they know it's wrong or else why have licence in their url03:15
wallyworld_sigh03:15
anastasiamacthumper: be original and say it in german03:16
thumper:P03:16
wallyworld_"es"03:16
wallyworld_or "der" or "sie" depending on gender03:16
wallyworld_did I pass?03:16
davecheneythumper: it's spelt COPYING03:18
thumperha03:19
menn0thumper: PTAL http://reviews.vapour.ws/r/175/03:30
thumpermenn0: kk, need to look at waigani's first03:30
menn0thumper: np03:31
thumperwallyworld_: wouldn't ya know it, juju was depending on that names package in juju/cmd03:46
* thumper will fix with the branch that increments the dependency03:46
thumperwallyworld_: and the backups command test checks for a wrong usage :-)03:47
wallyworld_\o/03:59
=== urulama_eod is now known as urulama
=== TheRealMue is now known as TheMue
anastasiamacericsnow: rbt problem.. probably related to the issue of creating my request yesterday... update did not occur automatically either. I had to manually update it...07:14
wallyworld_fwereade_: i have to go to soccer - do you have a little time to pop into #juju on canonical and offer some insight into some relation departed issues they are having in oil? some relations are behaving, others aren't, and i'm a little unclear on what to tell them07:22
fwereade_wallyworld_, I'll see what Ican do, thanks07:23
wallyworld_you rock07:23
mattywmorning everyone07:55
fwereade_mattyw, updated http://reviews.vapour.ws/r/234/diff/08:34
fwereade_mattyw, fwiw, it's almost all just moves08:34
fwereade_mattyw, the only things that should really be worth paying attention to are the code that's moved from uniter into context.factory; and the proxy junk that I deleted from uniter (which kinda comes under the same heading anyway)08:35
mattywfwereade_, ok great, I'll take a look this afternoon08:43
voidspacedimitern: ping09:04
dimiternvoidspace, pong, give me 20m please, doing a test now09:08
voidspacesure09:11
fwereade_does anyone know why we use an abstract socket for the jujuc server, but a filepath socket for the juju-run listener?09:40
fwereade_wallyworld_, when you return, ^^ -- on the basis that you're sort of near thumper and might know why he did it09:41
fwereade_gsamfira, does juju-run work on windows?10:26
fwereade_gsamfira, the tests have linux stuff hardcoded, I don't see how they can run10:27
gsamfirafwereade_: juju-run should work on windows, unless someone changed that code :). Instead of a unix socket, juju-run uses named pipes on windows10:28
fwereade_gsamfira, if the tests don't pass, I don't really consider it working ;p10:28
gsamfirathose tests have yet to be fixed on windows10:28
fwereade_gsamfira, what's the progress there?10:28
gsamfirayeah, a little more then half the tests pass on windows10:29
fwereade_gsamfira, because, you know, code without tests *is* fundamentally broken10:29
gsamfirafwereade_: my colleagues are working on the tests10:29
fwereade_gsamfira, in the same way that data without offsite backups only happens to exist by coincidence10:29
fwereade_gsamfira, I know, and I'm glad that they are, I'm just stressing about it a bit10:29
fwereade_gsamfira, do you have an ETA?10:30
gsamfirafwereade_: understood. There are some issues that are giving us a bit of a headache. For example, colon delimited files are not possible on windows10:30
gsamfiraso com.ubuntu.juju:series:foo10:30
fwereade_gsamfira, tedious -- which files are those?10:30
gsamfiracannot be written on windows10:30
fwereade_gsamfira, where would they come up? simplestreams?10:31
gsamfirafor those files some normalization needs to happen (the juju-metadata plugin fails for the same reasons)10:31
gsamfirayep10:31
gsamfirasimplestreams10:31
gsamfiraand a ton of tests rely on those10:31
gsamfiralet me ping bogdanteleaga to get the status of that10:32
fwereade_gsamfira, I strongly recommend you talk to wallyworld about that10:32
fwereade_gsamfira, gut feeling says there should be a simple way around that, and we shouldn't depend on those FS paths10:33
gsamfiraI think bogdan has been in touch with wallyworld10:33
fwereade_gsamfira, ok, fantastic10:33
gsamfiramost likely after the Paris ODS, I'll be joining in to work on the tests.10:34
gsamfirafwereade_: right now we are in crunch mode for the summit in openstack10:35
fwereade_gsamfira, fantastic -- just bear in mind that it's hard to legitimately claim windows support while the unit tests are still far from green10:36
gsamfirafwereade_: understood10:36
fwereade_gsamfira, would it be worth syncing up with (say) mgz to get proto-CI in place for windows? that way at least we can catch regressions in tests that had been passing10:37
gsamfirafwereade_: I would suggest we disable all tests that fail on windows ATM, and enable them as they get fixed10:37
gsamfirafwereade_: so yes. it is worth doing :)10:38
fwereade_gsamfira, mgz: ok, I'd be fine with a SkipWindows() func we jam into the failing tests, or something10:39
wallyworld_gsamfira: fwereade_ : there's a bug nate is aware of to change the file paths to be windows compatible10:39
fwereade_gsamfira, mgz: can I leave you guys to coordinate getting that done somehow?10:39
fwereade_wallyworld_, so we're reading dirs with tools in that format?10:39
wallyworld_fwereade_: thanks for helping with the relation stuff - i was not across those older bugs in previous versions10:39
fwereade_wallyworld_, ok, good to know10:40
fwereade_wallyworld_, yw :)10:40
wallyworld_fwereade_: we're reading dirs with file names containing :10:40
wallyworld_which is fine for linux10:40
fwereade_wallyworld_, yeah10:40
fwereade_wallyworld_, gsamfira: I guess I'm a bit surprised that the :s are problematic for a large proportion of the tests, though10:41
fwereade_wallyworld_, I can understand how they'd mess up the metadata ones10:41
wallyworld_fwereade_: so skimming the back scroll in #juju, their issues were caused by bugs in 1.16.5?10:41
fwereade_wallyworld_, I *think* so, yeah10:42
fwereade_wallyworld_, the units were coming up and believing they weren't in the relations they actually were10:42
wallyworld_fwereade_: gsamfira: many tests set up fake metadata, writing files with :, hence they fail10:42
fwereade_wallyworld_, which could happen pre 1.18.1, because we weren't recording what relations we'd joined locally until we saw a remote unit in them10:42
gsamfirafwereade_, wallyworld_ : bogdanteleaga proposed a fix for the : files10:42
gsamfirahttps://github.com/juju/juju/pull/90810:42
fwereade_wallyworld_, oh ok -- can we get around that by not writing the metadata to the filesystem?10:43
fwereade_wallyworld_, or just fixing the FS storage to map : paths to something else?10:43
fwereade_wallyworld_, hell just urlencide all paths?10:43
wallyworld_fwereade_: yes i think so, but there's a lot of tests to fix, and we need to change the file names anyway for the generate metadata plugin10:43
wallyworld_fwereade_: we originally used : because that's what the simplestreams folks did onthe public web server10:44
gsamfirafwereade_, wallyworld_  : https://github.com/juju/juju/pull/908 <-- I think this should do10:44
wallyworld_easy enough just to change the : to -10:44
wallyworld_gsamfira: ok, will look soon, gotta have dinner after coming hie from soccer10:45
wallyworld_home10:45
gsamfirawallyworld_: cool. If those 2 patches are fine, we can fix a large portion of the tests that try to write simplestreams files10:45
fwereade_gsamfira, wallyworld_: that CL scares the life out of me tbh10:45
fwereade_gsamfira, wallyworld_: way too many arbitrary-lookin changes in arbitrary-looking places10:46
fwereade_gsamfira, wallyworld_: why wouldn't just encoding internal paths work?10:46
fwereade_gsamfira, wallyworld_: the metadata fixes would be different but they feel like they really *are* different10:46
wallyworld_or just notusing filenames with : in them10:46
fwereade_gsamfira, wallyworld_: they're about parsing a particular FS structure10:47
wallyworld_the metadata is fine on either os - it's the filenames that need changing unless i am missing something10:47
fwereade_gsamfira, wallyworld_: why wouldn't we just make :s work in Storage paths?10:47
fwereade_gsamfira, wallyworld_: the solution to a leaky abstraction is to fix the leak10:47
wallyworld_fwereade_: the json files are written to disk10:47
fwereade_gsamfira, wallyworld_: not to add duct tape in 10 other files10:47
wallyworld_fwereade_: i've had no input to the CL, not seen it yet10:48
wallyworld_nor10:48
fwereade_wallyworld_, and? the Storage names can have :s just fine, we just need to encode them differently for the FS, surely?10:48
wallyworld_sure, but why do that? the file names are totally arbitrary10:48
wallyworld_just make file names compatible with all os's10:49
fwereade_wallyworld_, yes10:50
fwereade_wallyworld_, that shoudl be completely hidden behind the Storage interface10:50
wallyworld_yep10:50
wallyworld_but don't forget10:50
fwereade_wallyworld_, not splattered across the codebase everywhere we *happen* to have noticed a problem today10:50
wallyworld_we generate files on disk also10:50
wallyworld_we just need to change *1* method10:51
fwereade_wallyworld_, I understand that the metadata plugin is a separate issue10:51
gsamfiraaside from index.json, i *think* the rest can be named whatever we want. As long as we use the same path inside index.json10:51
fwereade_gsamfira, that is true as well10:51
gsamfirafwereade_: the metadata plugin uses the same filenames/constants. if we fix the metadata plugin, we fix most tests10:52
fwereade_gsamfira, wallyworld_: but the proportion of tests in which we depend on FS simplestreams data (as opposed to Storage-based simplestreams data) should be minimalk10:52
wallyworld_changing this const should work10:52
wallyworld_ProductMetadataPath = "streams/v1/com.ubuntu.cloud:released:imagemetadata.json10:52
wallyworld_fwereade_: agreed, i'm not sure of the current mix of tests which use storage vs fs metadata10:53
wallyworld_but regardless, changing a const should fix everything10:53
fwereade_sorry guys, my lunch is getting cold10:53
fwereade_wallyworld_, it wil *not*10:53
fwereade_wallyworld_, it will fix *one symptom* of the underlying problem10:53
fwereade_wallyworld_, which is that our FS storage abstraction is leaky and fails on windows10:53
wallyworld_ok, we can talk after your lunch10:53
fwereade_wallyworld_, cheers :)10:54
gsamfirawallyworld_, there is also this: https://github.com/juju/juju/blob/master/environs/tools/marshal.go#L2110:55
gsamfiraand the one immediately bellow that10:55
wallyworld_gsamfira: yes, i knew that was there but didn;t find the method in time to type it in10:55
wallyworld_there's a couple of places where we generate the filename10:55
gsamfirawallyworld_: if we can create a constant with those names, that would be great10:57
gsamfirabogdanteleaga: can you try changing the filenames and see if that works on both windows and linux ?10:57
wallyworld_gsamfira: the content id doesn't need to change10:58
wallyworld_just the file name10:58
bogdanteleagathere's quite a lot of places where there are :'s, mostly are sptrintf'd10:58
bogdanteleagaand we have some tests where they are hardcoded10:58
wallyworld_it's only the file names isn't it?10:58
bogdanteleagabut it's pretty doable10:58
bogdanteleagayes10:58
wallyworld_the content ids should be fine10:58
gsamfiracontent ids are fine, yes10:58
wallyworld_so there should be a small number of places where filenames are generated, these just need to have the : replaced10:59
gsamfiraonly paths inside index.json (and of course the names on disk) need to change10:59
bogdanteleagaand what about files online?11:00
bogdanteleagathere's no difference between the offline and offline paths, they're generated from the same string11:00
gsamfirabogdanteleaga: those can be replaced. the index.json is the only one of consequence, and we don't touch that11:00
gsamfiraso the path to the file containing information about tools in in the index.json file11:01
wallyworld_the online ones are url paths11:01
gsamfirahttp://paste.ubuntu.com/8639101/11:01
wallyworld_it's just windows fs that is broken :-(11:01
gsamfirathe "path" inside this file tells you where to find the tools information.11:02
gsamfirawallyworld_: yeah...unfortunately...windows uses ":" for drive letters....11:03
wallyworld_stupid windows, stupid drive letters11:03
gsamfirawallyworld_: ....amen11:04
gsamfirawallyworld_: its funny because windows also has UNC paths, which would be nicer to use in general...11:06
wallyworld_yeah, go figure11:08
perrito666morning11:49
TheMueperrito666: morning11:52
perrito666natefinch: https://github.com/juju/juju/pull/928 care to take a second look?12:42
jcw4fwereade: question about timestamps in action docs12:44
jcw4fwereade: creation time and completion time are straightforward12:44
jcw4fwereade: capturing start time (i.e. when the action is picked up and started) is a little tricky.12:45
jcw4fwereade: we chose to only write actionDoc once when enqueing it, and then writing a new actionResultDoc when done12:46
jcw4fwereade: so that the watchers can be efficient about detecting new actions, and not fire when we *update* the actionDoc12:47
jcw4fwereade: I'm just bringing this up 'cause you'd mentioned capturing start time, and I'm looking for your preference on how to capture that12:47
fwereadejcw4, can we plausibly write the beginnings of the actionResultDoc when we start?12:49
jcw4fwereade: that's the only thing that makes sense to me12:49
fwereadejcw4, the big trouble is that we still don't seem to have proper handling for units that die halfway through an action12:49
fwereadebodie_, was there any progress there?12:49
jcw4fwereade: I think we'd have to rework how we write actionResultDoc though, since I think the code expects it only to be written once too, on completion.12:50
jcw4it actually might be cleaner, if we begin actionResult right away and remove actionDoc as soon as it's picked up, but we'd have to decide what happens when units die halfway through.12:52
jcw4at least this way it'd be clear from the actionResultDoc that a unit tried to start working on it12:52
fwereadejcw4, yeah, I think that would be handy12:53
fwereadejcw4, the uniter has responsibilities if it goes down mid-action12:53
fwereadejcw4, but if it goes down forever, we should also have the state-server-side cleanup for a dead unit explicitly mark all those actions failed12:53
fwereadejcw4, the uniter responsibilities are one of my major concerns, though12:53
fwereadebodie_, ^^12:53
jcw4fwereade: do we already have mechanisms in place for uniter cleanup ?12:54
jcw4before the big hammer of state-server-side cleanup12:54
fwereadejcw4, we're *meant* to12:54
fwereadejcw4, everything the uniter does can pick itself up and recover if it's kill -9'd at any time12:55
fwereadejcw4, *except* actions12:55
jcw4ooh12:55
fwereadejcw4, to be fair, it's pretty easy12:55
fwereadejcw4, it just needs to detect and handle it in ModeContinue12:55
jcw4fwereade: that's partially because we don't have a way right now to know that actions have been started... which an initial record of actionResult may help.12:56
fwereadejcw4, I'm inclined to make it local, actually12:56
jcw4hmm; interesting12:56
fwereadejcw4, we do already record that fact locally12:56
jcw4i see12:56
fwereadejcw4, just got factored out into uniter/operation package12:56
jcw4fwereade: I saw your emails about that refactor, but haven't looked at the diff12:57
jcw4fwereade: I'll check it out too12:57
fwereadejcw4, the reason we no longer run actions in error states is because the current implementation smashes op-error state12:57
fwereadejcw4, this does need to be fixed :)12:57
jcw4fwereade: the current implementation of action handling in uniter you mean?12:58
fwereadejcw4, yeah, it's just writing out hook state12:58
fwereadejcw4, which smashes pre-existing hook-pending state12:58
jcw4fwereade: ah; I see.12:59
fwereadejcw4, which is how the uniter can tell it was interrupted running one, and hence should be in error mode12:59
fwereadejcw4, I did agree that it's not *top* priority -- "can't run actions in error state" is a bug, but it's not unreasonable in the first cut, while "running actions in error state messes everything up" is unreasonable at any time13:00
jcw4fwereade: +113:02
jcw4fwereade: also; slight side topic:  I'm *really* uncomfortable embedding time.Now() in the bowels of any state code.. I'd want to inject the concept of current time somehow to make chronology more predictable, and more testable.13:02
fwereadejcw4, that sounds good to me13:02
fwereadejcw4, take a look at the metrics stuff13:02
fwereadejcw4, I can't remember how they do that13:03
fwereadejcw4, if it's good, do the same13:03
jcw4fwereade: okay - I did see some code there13:03
fwereadejcw4, if it's not, please fix both ;p13:03
fwereadejcw4, and maybe sling thumper a mail? I suspect he is, or soon will be, doing something like that re user stuff13:03
jcw4fwereade: what I saw there looked like 1/2 a solution of at least capturing current time in a function that could be monkey patched13:04
jcw4It seems like a TimeProvider interface or something would be good13:04
fwereadejcw4, +1 to that13:04
fwereadeaxw, ping13:04
mattywdimitern, are you able to do a trivial review? http://reviews.vapour.ws/r/248/13:11
dimiternmattyw, sure, looking13:12
jcw4mgz, sinzui any idea why CI keeps failing on Does not match ['fixes-1384175'] ?13:28
jcw4that bug doesn't show up in the critical bugs on lp13:29
jcw4https://bugs.launchpad.net/juju-core/+bugs?field.searchtext=&orderby=-importance&field.status%3Alist=NEW&field.status%3Alist=CONFIRMED&field.status%3Alist=TRIAGED&field.status%3Alist=INPROGRESS&field.status%3Alist=INCOMPLETE_WITH_RESPONSE&field.status%3Alist=INCOMPLETE_WITHOUT_RESPONSE&field.importance%3Alist=CRITICAL&assignee_option=any&field.assignee=&field.bug_reporter=&field.bug_commenter=&field.subscriber=&field.structural_sub13:29
jcw4or13:29
jcw4https://api.launchpad.net/devel/juju-core?ws.op=searchTasks&status%3Alist=Triaged&status%3Alist=In+Progress&importance%3Alist=Critical&tags%3Alist=regression&tags%3Alist=ci&tags_combinator=All13:29
jcw4mgz, sinzui bug 1384175 shows as fixed in LP13:30
mupBug #1384175: Utopic test failures due to addition of vivid series <ci> <regression> <test-failure> <vivid> <juju-core:Fix Committed by thumper> <juju-core 1.20:Triaged by anastasia-macmood> <https://launchpad.net/bugs/1384175>13:30
mgzjcw4: it's only fix committed atm13:31
mgzit requires fix released13:31
mgzwhich I will mark now, as the utopic unit tests are blue13:32
mgzjcw4: go wild13:32
jcw4mgz: I see; I need to update my bookmark for checking blockers :)13:32
jcw4woo hoo13:32
sinzuimgz, jcw4 : We are finishing of the test run and we can see the test will be declared a pass. I will mark the bug as fix released13:34
jcw4sinzui: ta (mgz beat you to marking the bug as fix released though)13:35
=== ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: None
sinzuinatefinch, is anyone working on a backport of the vivid and ppc64el test fixes to 1.20?13:46
natefinchsinzui: yeah, me or someone to whom I delegate13:48
* perrito666 ducks13:48
sinzuinatefinch, :) I never assume it is you when I ask, I just hope you know the answer13:48
natefinchsinzui: probably a good assumption ;)13:49
ericsnownatefinch, perrito666: standup?14:06
perrito666ericsnow: natefinch just moved it14:06
perrito666you should have a new invite14:06
natefinchericsnow: sorry... there's a tosca meeting today that always conflicts, I should have moved today's meeting like 2 months ago14:07
gsamfirafwereade, bodganteleaga: so there are essentially 2 major aspects that need to get fixed to have all tests that require to generate/upload tools to work (and also fix the metadata plugin). First one is the obvious problem of having colons in filenames written to disk. The other is the file:// handler the http module uses to fetch local files.14:07
ericsnownatefinch: forgot to refresh my calendar14:08
gsamfirafwereade, bodganteleaga: changing the filename that gets written to disk should not impact functionality. We can do this for bith windows and linux. What is a bit wonky is the file:// handler14:08
fwereadegsamfira, ah, so file:// urls won't work right on windows?14:09
natefinchfile:// works fine on windows actually14:09
gsamfirathey do work, but not the way juju uses them14:09
natefinchheh14:09
fwereadegsamfira, I'd be entirely happy to have FileStorage run an http server and just happen to use an FS backend14:09
gsamfirafwereade: on linux we register '/' as the root for file://14:10
fwereadegsamfira, I think we already have an HTTPStorage like that anyway?14:10
natefinchfwereade: do we really want to change the whole way filestorage works just to avoid using colons?  I think getting this done sooner rather than later is more important than making it perfect.14:10
gsamfirafor that bit, it would be ok to use a small http server for tests only14:10
gsamfiraBUT, there comes the need to generate simpolestreams files using the client14:10
gsamfirafwereade: for example, the juju-metadata plugin14:11
fwereadegsamfira, sure, understood14:11
gsamfirathat plugin uses file:// iirc14:11
fwereadegsamfira, my position there is that we have a *huge* number of tests not passing thanks to FileStorage14:11
fwereadegsamfira, which equates to a huge proportion of juju not working, or at best working by coincidence14:11
gsamfirahuge is right :)14:11
fwereadegsamfira, so fixing filestorage is the important thing14:12
fwereadegsamfira, because we believe that's just a test problem14:12
gsamfirafwereade  : *local* file storage14:12
fwereadegsamfira, metadata doesn't work anyway14:12
fwereadegsamfira, and IMO it strictly comes behind fixing the tests that we believe are spurious failures14:12
fwereadegsamfira, certainty on the bits that do work comes first14:13
gsamfirafwereade: there is another option. The way file:// is mapped now, assumes that we have a root folder. On linux using "/" is fine, because any volume is mounted under that. On windows we have drive letters (unfortunately). So A possible solution would be to implement a version of http.NewFilesystemHandler() that does not chroot all requests inside a folder14:14
gsamfiraand simply does a os.Stat()14:14
gsamfiraif the file is local, just serve it14:14
gsamfirainstead of doing filepath.Join(rootDir, filename)14:14
gsamfiraand then serving it14:14
gsamfirathen we would be able to have file://C:/my awesome/file14:15
katcocan i get a quick lgtm on a 2-line change? http://reviews.vapour.ws/r/236/diff/#14:15
gsamfirakind of the same way every other browser on the planed does it14:15
gsamfira:)14:15
fwereadegsamfira, bogdanteleaga: do we need the full path there anyway? since we register the handler anyway, can we not just have file:// defined as relative to rootDir?14:15
gsamfirawe can, but then we need to strip the drive letteer every time. Juju generates full paths most times14:16
gsamfiraso juju will most times pass: "C:/path/to/file"14:16
gsamfirafwereade: so paths would become file://C:/path/to/file14:17
fwereadegsamfira, why would we need to strip the drive letter other than inside FileStorage?14:17
gsamfiraif we chroot NewFilesystemHandler to C:/14:17
gsamfiraand pass in C:/path14:18
gsamfirafile would look for C:/C:/path14:18
fwereadegsamfira, why would we be passing in C:/anything?14:18
gsamfirafwereade: because that's how juju generates all paths :)14:18
fwereadegsamfira, all I care about is that URL() returns something that's a valid url, that we can write a handler for that goes back to the right place14:18
fwereadegsamfira, er, what?14:18
fwereadegsamfira, the Storage interface has complete control14:19
fwereadegsamfira, the fullPath thing looks like the problem here14:19
fwereadegsamfira, that's how FileStorage works now14:19
fwereadegsamfira, but it evidently doesn't work right on windows14:19
fwereadegsamfira, so lng as we end up with (1) FileStorage working on windows and (2) none of the client code having to dick around with windows-isms to use FileStorage, I'm happy14:20
gsamfiracool14:20
fwereadegsamfira, in fact, (2) beats out (1)14:23
fwereadegsamfira, if we can't eliminate the windows-isms, I'd be perfectly happyto see HTTPStorage used instead14:24
fwereadegsamfira, tedious that it'd need cleanup14:24
fwereadegsamfira, but really, whatever minimal-friction Storage interface we can get is what we need14:24
gsamfirafwereade: so then what needs to happen now is: 1) have fullPath return a correct path on windows 2) chroot in C:/ on windows (for now) 3) filenames need to contain valid characters on both OS14:24
fwereadegsamfira, I think I'm suggesting that we should drop the notion of fullPath entirely14:25
bogdanteleagafwereade: are you talking about the URL functions in environs on (2)?14:25
fwereadegsamfira, register the stupidTestFile:// handler if we have to14:25
fwereadegsamfira, nobody should be using file:// urls in anything other than a test context anyway afaics14:25
fwereadegsamfira, except for wget in that special cloudinit case, which I don';t think is underconsideration here14:26
fwereadegsamfira, is it?14:26
gsamfiranope14:26
katcojam: hey i see you're ocr, do you have time to give a quick look at a 2-line change? http://reviews.vapour.ws/r/236/diff/#14:26
fwereadekatco, not 100% sure about that -- I think we want to prioritise speed in the local provider, and for thumper to finish his local provider plugin that will give us better handling of image updates14:38
fwereadekatco, am I misunderstanding?14:39
katcofwereade: i am definitely getting mixed messages on this14:39
katcofwereade: originally that was what we were going for14:39
sinzuinatefinch, https://bugs.launchpad.net/juju-core/+bug/137014914:39
mupBug #1370149: No defined 'state-servers' on environment file after bootstrap, works after run 'juju status' <bootstrap> <cloud-installer> <cts-cloud-review> <juju-core:Triaged> <https://launchpad.net/bugs/1370149>14:39
katcofwereade: but wallyworld_ asked me to make this change, and i think there are a few others who would like to see this behavior14:39
fwereadekatco, dammit14:40
fwereadekatco, do you know where the requirement came from upstream of wallyworld_?14:40
katcofwereade: it can sit there a bit longer until there's some consensus. i'm only trying to finish it on wallyworld_'s behest before the eow14:40
katcofwereade: let me do some digging rq14:41
fwereadekatco, cool, thanks14:41
fwereadekatco, the code looks fine fwiw14:41
wwitzel3fwereade: ping, and I good to start on the juju-run side of things?14:47
wwitzel3s/and/am14:47
fwereadewwitzel3, I've been hoping for a review of http://reviews.vapour.ws/r/234/14:55
fwereadewwitzel3, if I can hassle you for a review of that, and maybe 1 followup, you should be good to go14:58
fwereadewwitzel3, they're both a lot simpler than they look, they're really just moves...14:58
voidspaceTheMue: so you point out in your review that the test doesn't use the constant for "none"15:04
voidspaceTheMue: this is because the constant is not available in the test namespace15:04
voidspaceTheMue: but it isn't a potential source of error15:04
voidspaceTheMue: if the string literal is wrong then the test will fail15:04
voidspaceTheMue: the same applies for the "default-vpc"15:04
voidspaceTheMue: so I don't think there's much value in defining constants just for those test uses15:05
voidspaceTheMue: as they provide no actual value15:05
voidspace(s/namespace/package/)15:05
TheMuevoidspace: ok, can live with it. only always get a bit scary of typos when seeing repeating strings15:06
voidspaceTheMue: I understand15:06
voidspaceTheMue: but for a single use string literal (the test "none") the const is just as likely to have the typo... and it's now non-locally to the test15:06
voidspaceTheMue: so you're even less likely to see it!15:06
voidspaceTheMue: more importantly though, for these literals, a typo will cause an actual failure15:07
voidspaceso I don't think there's a danger in *these* cases15:07
TheMuevoidspace: yep, in your case it's ok.15:07
voidspaceTheMue: thanks, can I have a "ShipIt"?15:07
TheMuevoidspace: didn't I already gave you one?15:09
* TheMue -> looking15:09
voidspaceTheMue: I didn't think so15:09
voidspaceI may just have not seen it :-)15:09
TheMuevoidspace: it has been a fix it, then ship it. now it's only a ship it. :)15:10
voidspaceHaha, ok thanks15:10
* fwereade has frightened wwitzel3 away :(15:22
voidspacethat's an achievement15:22
voidspacehe's pretty hard to frighten15:22
fwereadeok, cath will be justifiably upset with me if I don't go and see the blue lagoon before the sun sets15:23
fwereadeI hope I will make it back on tonight15:24
fwereadebut then I meant to do that yesterday, and ended up sleeping 12 hours or something15:24
voidspacefwereade: o/ night15:26
voidspacefwereade: enjoy the lagoon :-)15:26
fwereadevoidspace, cheers15:27
voidspacePANIC: machine_test.go:70: MachineSuite.TestSetRebootFlagDeadMachineRace15:27
fwereadenatefinch, I appoint you answerer of questions while I'm out ;p15:27
voidspace... value *errors.Err = &errors.Err{message:"", cause:(*net.OpError)(0xc210362380), previous:(*errors.Err)(0xc21041f690), file:"github.com/juju/juju/state/open.go", line:68} ("cannot create log collection: local error: bad record MAC: local error: bad record MAC")15:27
voidspace^^^ familiar anyone?15:27
natefinchvoidspace: wacky...15:32
voidspacenatefinch: that was a buildbot failure for my merge15:32
voidspacedidn't fail locally15:32
voidspacewill retry15:33
natefinchvoidspace: that's a tls error... almost certainly not a code problem15:34
perrito666voidspace: saw it then when I tried to reproduce it it was gone15:34
voidspacenatefinch: perrito666: thanks15:35
mgzgsamfira: do you have a mo, I'm trying to understand some kardianos/service dep weirdness16:06
gsamfiramgz: sure. I will gladly help if I can16:07
natefinchmgz: doh, is it causing a problem?  I just changed the deps yesterday so godeps -t ./... would produce the right output16:07
mgznatefinch: maybe? I didn't see your change *doing* anything to the windows deps though16:08
mgzit just seems like kardianos/service/service_windows.go needs code.google.com/p/winsvc - which we don't have as a dep16:08
mgzbut this *did* build before, and I can't see what has changed16:09
mgzgsamfira: so, do we need to build that file, and is it actually a dep?16:10
gsamfiramgz: the kardianos/service is actually a wrapper for winsvc (on windows)16:11
gsamfiraso yeah16:11
natefinchweird that godeps isn't picking that up16:12
natefinchahh... I see, it's only included in an _windows.go file16:13
natefinchI'm surprised it built before if we weren't including that package.  Do we build the windows code differently?16:16
mgznatefinch: not deliberately, but perhaps16:18
mgzso, I'll just file a bug, and we add the dep now I guess16:18
natefinchyeah16:18
* natefinch grumbles about not using go get16:18
mgzit does, but we try and make our tarball make sense, mostly for ubuntu's benefit (they have to document all the licences of all the deps and such like)16:20
mgzso, this is a bit of an edge case in that they don't really need to care about windows-only dependencies, but life is still somewhat easier if we actually know what our code needs to build16:20
jamespagedimitern, hey - just noticed that juju 1.21 started accessing the bootstrap node via its IPv6 address ;-)16:34
dimiternjamespage, hey, yeah - esp. if you set prefer-ipv6: true in env.yaml :)16:37
jamespagedimitern, ooooooo16:37
dimiternbut apart from that, prefer-ipv6 does not do much16:38
dimitern(yet)16:38
mgzgsamfira: filed bug 138481816:49
mupBug #1384818: Windows service dependency missing <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1384818>16:49
mgzwe probably also want to change the 1.20 branch16:49
sinzuinatefinch, mgz, do either of you have a minute to review https://github.com/juju/juju/pull/95517:35
natefinchsinzui: LGTMd17:44
sinzuithank you natefinch17:44
voidspaceg'night all18:10
natefinchthis is why I never write bash scripts.... because stuff that looks like it should absolutely work, just doesn't18:38
natefinchwtf is wrong with this: #!/bin/bash18:39
natefinchexport GOPATH=$HOME18:39
natefinchgo get gopkg.in/yaml.v118:39
natefinch^^^ says GOPATH is not set18:39
natefinchhow is that possible?18:39
perrito666natefinch: its most likely your fault :p18:58
perrito666natefinch: pastebin the whole thing18:59
natefinchhttp://pastebin.ubuntu.com/8644139/19:00
natefinchit's an install hook where I tried to do the "right thing" by using go run.  But seriously... this is why I never write bash scripts19:00
perrito666natefinch: I noticed19:00
perrito666natefinch: you can ditch the export19:00
natefinchok19:00
natefinchis that what's screwing me?19:00
perrito666nope19:01
natefinchlol19:01
perrito666although you are assuming that $HOME actually has something19:01
perrito666since its bash you can safely use ~19:01
natefinchyeah, I thought of that19:01
perrito666mm, re thinking keep the export I am not sure what go get does inside19:02
thumpermorning folks19:06
perrito666morning t19:06
perrito666meh, too many t people19:06
TheMuethumper: morning19:06
thumperhmm...19:40
thumperboth the on call reviewers for Friday still think it is Thursday19:41
thumper:-(19:41
thumperthat wasn't well planned19:41
perrito666thumper: well I also think there is some thursdayness in the day19:42
perrito666the fun part is that both thursday reviewers think its friay19:43
perrito666friday*19:43
thumperanyone? http://reviews.vapour.ws/r/254/diff/19:45
ashipikahttp://reviews.vapour.ws/r/247/19:45
perrito666thumper: done19:49
thumperperrito666: ta19:49
menn0thumper: I've just responded to your review comments for 17520:51
menn0thumper: one thing though, can you explain why it's preferable to use a non-LTS series for machines in tests?20:51
thumpermenn0: histerical raisins20:52
thumperI'm sure there was a rational reason at some stage20:52
thumperbut I don't recall what it was20:52
menn0thumper: ok np20:52
menn0thumper: gah! I've just noticed that envUserDoc uses "envuuid" instead of "env-uuid"20:54
thumper:-(20:54
menn0thumper: we've been using env-uuid for the migrated collections20:54
thumperwhich is good20:54
menn0thumper: should we fix now while it's new?20:55
thumperI think the dashes make it more readable20:55
menn0thumper: I noticed because I was thinking about machineid vs machine-id20:55
menn0thumper: I think we should settle on machine-id and make the pre-existing fields consistent20:56
thumper+120:56
menn0thumper: what about envUserDoc? that's just been released hasn't it?20:58
thumperwe should migrate it, not just assume20:58
menn0thumper: ok20:58
menn0thumper: I'll do that as a separate branch20:59
menn0thumper: onyx standup hangout?21:01
thumpersure, there in 2 m21:01
=== arosales_ is now known as arosales
wallyworld_thumper: hey22:33
thumpero/22:33
wallyworld_a question when you were offline22:33
wallyworld_<fwereade_> does anyone know why we use an abstract socket for the jujuc server, but a filepath socket for the juju-run listener?22:33
wallyworld_answer?22:33
thumperif there was a reason, I don't recall it now22:34
wallyworld_ok :-)22:34
thumperI do think there may have been a reason22:34
thumperactually I think I do know22:34
wallyworld_next time you "see" william, maybe touch base with him about it22:34
thumperjujuc used to use a filepath one too22:34
thumperperhpas this was changed for the windows work22:35
wallyworld_ah ok22:35
thumperwithout them realisign about the juju-run one22:35
wallyworld_makes sense22:35
thumpermenn0: have you updated the rb diff with the latest stuff?22:58
thumpermenn0: although I have noticed that when you push to github, it gets the diff updated22:58
thumpermenn0: I'm still seeing "machineid" and I thought you were changing22:59
menn0thumper: nope. just finishing off the bit to move the upgrade steps to 1.21alpha322:59
thumperok...22:59
thumperI'll go to exercise before reviewing then22:59
thumperwallyworld_: can we chat this afternoon about the local charmstore integration?22:59
menn0thumper: the tests in the upgrades package were lacking so I'm fixing them too22:59
menn0thumper: give me 10 mins22:59
thumpernp22:59
thumpermenn0: take an hour :)23:00
menn0thumper: I have to take my son swimming soon so it'll be much less than that23:00
menn0thumper: done23:07
wallyworld_thumper: sure, just ping when you want to talk23:19

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