[05:41] Anyone awake? [05:56] jpds: I am awake, something I can help with? [06:05] hello. I appear to be having the toughest time trying to connect to my swift object-store on devstack via juju [06:05] ive pulled up the url of the object-store via keystone catalog [06:06] however, once i create the juju-dist and upload the meta-data [06:06] i still get an un-authorised connection [06:06] ideas? [07:16] mornin' all [07:18] hmm, i left juju running the local provider over friday and saturday and ended up with a 2.1GB log file [07:20] ha, it's all my fault [07:21] it panicked 47717 times before i killed it yesterday [07:22] morning rogpeppe [07:22] heh :) [07:22] axw: hiya [07:26] axw: i was just glancing at https://codereview.appspot.com/14465045/ [07:26] axw: where are the sftp urls coming from? [07:26] rogpeppe: environs/sshstorage [07:26] its URL method returns sftp:// URLs [07:26] axw: ah, because the URLs were only expected to be used from a shell script? [07:26] rogpeppe: I was expecting them to not be used at all (or only for logging) [07:26] axw: ah, i see [07:26] but yeah, they get used by wget or http.Get in some cases [07:26] axw: also, not quite sure what you mean by "problematic for the trunk implementation" [07:26] axw: do you just mean the trunk branch of juju? [07:26] rogpeppe: yeah, sorry, badly worded [07:26] I just mean that the current code is broken [07:26] and this change fixes it by wrapping it in httpstorage [07:26] axw: np, i *thought* so, but i wasn't sure [07:26] thus hiding the invalid URL method away [07:27] axw: right, now i think i have enough info to review the change properly :-) [07:27] :) thanks [07:28] axw: hmm, i'm getting a chunk mismatch error [07:28] axw: could you repropose please? [07:28] yup, just a moment [07:34] rogpeppe: do you think it'd be worthwhile using gotype instead of "go build" in .lbox.check? might speed things up a bit ... can you think of any downsides? [07:34] maybe it's just my crappy laptop's the problem [07:34] axw: is there a gotype command? [07:34] (repropose is still chugging along) [07:34] rogpeppe: yeah, somewhere in go.tools [07:35] axw: you're running go tip then, right? [07:35] code.google.com/p/go.tools/cmd/gotype [07:35] rogpeppe: nope, why? [07:35] go/types works on 1.1 [07:35] it didn't for a bit, but rsc fixed that [07:35] axw: hmm, what go version are you using? [07:35] rogpeppe: 1.1.1 [07:35] axw: axw: i'm pretty sure it's not go build that takes all the time [07:35] axw: hmm [07:36] axw: on my machine it's go vet that takes forever [07:36] rogpeppe: I didn't even verify. okay, probably not worthwhile then [07:37] axw: but... [07:37] axw: i thought that was only since go vet became external (since go 1.1.1) and started using go types [07:37] axw: it takes about 30s to run on my machine [07:38] rogpeppe: minutes here, but I have a crappy laptop [07:38] rogpeppe: propose finished [07:38] axw: i mean "go vet" takes 30s [07:38] axw: propose can take minutes for me too [07:38] hmmk [07:39] I suppose it doesn't matter too much, forces me to think a bit harder before proposing junk ;) [07:44] axw: i really wish it was faster [07:44] axw: BTW i'm *still* seeing a chunk mismatch :-| [07:45] ack [07:45] :( [07:45] hmm [07:45] morning [07:45] rogpeppe: where does this chunk mismatch show up? [07:46] morning TheMue [07:46] axw: https://codereview.appspot.com/14465045/diff/1/provider/null/environ.go [07:46] axw: one mo, maybe i didn't reload the main CL page [07:46] axw: ignore me [07:46] rogpeppe: I think so [07:46] nps [07:46] axw: it works fine [07:47] axw: does anything use sshstorage other than the null provider? [07:47] rogpeppe: nope [07:47] only the null provider, only for bootstrap [07:51] axw: oh yes, one thing that's been niggling at me for a while: how do you ensure that the bootstrap storage object is eventually closed? [07:51] axw: as doesn't it contain a live ssh connection? [07:51] rogpeppe: you don't/can't at the moment [07:51] yep [07:52] axw: ah, so it just leaks [07:52] rogpeppe: yes, but this is only ever done from the command line [07:52] axw: currently... [07:53] hmm [07:53] axw: we shouldn't assume that though - it's ok for people to use this stuff directly from Go. [07:53] rogpeppe: if this is moved to something non-interactive, I think we'll have bigger problems [07:53] like, how to input sudo passwords [07:53] axw: ha, yes [07:53] but yeah I understand [07:53] axw: although that's not a problem if it's already running as root, i guess [07:54] true [07:54] actually [07:54] not true [07:54] it's the target machine that matters [07:54] and you typically wouldn't allow root ssh [07:55] axw: how does the target machine know you're running as root? [07:55] rogpeppe: anyway, I guess this would be solved by adding a Close method to Environ [07:55] rogpeppe: ? [07:55] axw: yes, but that's actually a very big change [07:56] rogpeppe: null provider bootstrap takes two parameters: host, and login user [07:56] axw: well, actually, it would probably need a close method on both Environ and Storage [07:56] rogpeppe: login user is the current user on the source machine by default, but you'd probably need to change that to something non-root [07:57] axw: sure [07:59] axw: we could probably use a comment in the code just to let people know that this has been thought about [08:00] rogpeppe: okey dokey, will add one [08:05] morning folks, any reason why the maximum length of a summary using lbox is so low? [08:06] mattyw: i think it's an lp restriction [08:06] mattyw: ridiculous, ain't it? [08:07] axw: just to clarify: what code is actually using the storage URL? [08:08] rogpeppe: I didn't follow all the way to the end, but it's related to generating simplestreams metadata [08:08] if the tools exist on the target, but the metadata doesn't, it grabs the tools off the target and generates the metadata from them [08:09] axw: ah! so it uses the URLs inside the metadata [08:10] rogpeppe: it uses them to obtain the tools, to generate the metadata [08:10] axw: probably the other way around, no? it doesn't need to obtain the tools to generate the metadata, but the metadata will contain URLs which need to point to the tools [08:10] axw: (but i haven't looked at the code...) [08:11] rogpeppe: I think the juju CLI is fetching them off the target, generating the metadata at the CLI end, then uploading the metadata [08:11] because there's nothing running on the target necessarily [08:11] (in this case there isn't) [08:11] the metadata includes information about the tools like file size and SHA256 sums [08:11] so the tools need to be obtained to generate it [08:14] axw: hmm, so how can tools exist on the target without metadata? i thought we'd moved away from legacy tools storage... [08:15] rogpeppe: I could only surmise that it was a botched bootstrap [08:16] it got part way through downloading the tools, but didn't get as far as generating/uploading the metadata [08:16] rogpeppe: then a second bootstrap found the tools on the remote machine, but no metadata [08:16] so it decided to fetch them and generate the metadata then (and failed) [08:16] axw: hmm [08:17] axw: perhaps that's the logic that needs to be fixed [08:17] rogpeppe: perhaps, though I think it's prudent to hide invalid URLs away anyway [08:18] something else may come along wanting to use it [08:18] axw: well, the difficulty i have with this is that URLs are supposed to be usable from other machines [08:18] though TBH, it's still flawed - they're only valid for the lifetime of the current process [08:18] axw: and this only makes sense if the URL is only used locally [08:18] * axw nods [08:37] fwereade, I'm about to set that mp as needing review, are there any bugs or blueprints I should link it to? [08:38] mattyw, nothing for that work specifically -- IIRC there's an "identity" blueprint somewhere that the followups will probably apply to though [08:39] mattyw, if you'd like to capture your requirements as a bug and link that it can't hurt though [08:39] mattyw, always nice to be able to look back and figure out why we write the code in the first place ;) [08:40] fwereade, ok will do [08:41] axw: i'm having difficulty finding the logic that fetches the tools off the target, generating the metadata at the CLI end [08:42] axw: istm that to do that it would need to List the storage, but there are very few places that do that [08:43] rogpeppe: I'll have a hunt [08:43] rogpeppe: in the mean time, https://codereview.appspot.com/14483043 [08:44] axw: argh, darn chunk mismatch again [08:44] ah wtf [08:45] rogpeppe: the copying/listing/etc. is in environs/sync [08:45] brb [08:51] fwereade, https://codereview.appspot.com/14389043/ thanks :) [08:51] rogpeppe: in environs/tools/simplestreams.go, generateMetadata [08:51] " if fetch && t.Size == 0 {" [08:52] axw: yeah, i'm seeing it now [08:52] axw: in sync.SyncTools [08:53] mattyw, tyvm [08:53] fwereade, no, thank you [08:53] rogpeppe: ideally that code should use the Storage [08:53] rogpeppe: but that would require a means of getting from a URL back to a storage name [08:54] axw: hmm, yeah [08:55] axw: unless a tools.List contained an expanded version on Tools which also held the original name, i guess [08:56] yeah, or that [08:57] rogpeppe: I reproposed that CL [08:57] * fwereade breakfast [09:11] axw: istm that part of the problem is that we're in a half-way house between two schemes - we've got two forms of metadata (the simple streams stuff and the storage list) and the latter doesn't have enough information to fulfil the former's requirements [09:12] axw: that is, the real problem here is that toosl.ReadList isn't returning information on the hashes & sizes of the contents of the storage [09:13] rogpeppe: yeah, because that would incur quite a lot of overhead for the usual case where the metadata already exists [09:13] but yes, there's information loss there [09:14] the storage names in particular [09:18] axw: what would happen if we just passed in fetch==false ? [09:20] rogpeppe: the metadata would be written out with Size=0, and no hash. I *think* jujud gets unhappy about that [09:20] axw: that's what i was wondering [09:23] axw: it's good to think through stuff in this area, i think, especially because we want to solve lp:#1219582 [09:23] <_mup_> Bug #1219582: bootstrap is slow, lookups tools multiple times

[09:25] it's kind of ironic that s3 does actually provide hash and size info, although i think it's only the md5 hash [09:30] axw: does it actually make a difference if the metadata exists on the target machine? [09:31] axw: it looks to me as if the logic in SyncTools ignores any metadata in the target [09:32] rogpeppe: it ignores it for copying the tools over, but it's used to decide which tools tarball to install into an instance... not sure what happens if it's not there [09:32] axw: which means that we'll always read all the tools in both the target and source storage; i may have missed something though. [09:32] * axw_ tries === axw_ is now known as axw [09:32] rogpeppe: that doesn't sound right, let me check [09:35] fwereade, ping? [09:35] rogpeppe: SyncTools only does a storage.List on the source and target, and copies across what's missing from source. It grabs metadata from target if it exists, then generates metadata for what's missing by fetching the tools back off the target [09:35] axw: ah, no WriteMetadata does it, i see [09:35] axw: we really need some sort of local storage cache [09:36] mattyw, pong, sorry, got distrcted half way through [09:36] rogpeppe: yeah, that could certainly help [09:36] fwereade, no problem, just going through the next stage of the id stuff you said about adding the owner as an argument to AddService [09:37] mattyw, ah, yes [09:37] fwereade, that seems to break A LOT of stuff (all just mechanical changes because now there's a new argument) so I just wanted to check that this was what you wanted [09:39] mattyw, I can't think of another way to do it, I'm afraid [09:39] fwereade, ok no problem, just wanted to check we had the right idea before we fix all the things we broke [09:39] mattyw, yeah, thanks for checking :) [09:40] fwereade, I guess this is one reason for it being the next step - the number of lines in the diff is likely to be huge [09:41] mattyw, indeed, probably best to do as little as possible beyond that [09:48] mattyw, fwereade: this may be a silly idea, but i just thought i'd mention the possibility: [09:48] mattyw, fwereade: rather than add "owner" arguments to lots of methods in State, we could potentially have a facade in front of State that knows the current owner [09:49] rogpeppe, we're not planning to give anything except services owners at the moment [09:50] fwereade: even still - i just wonder if it might be a more economical approach, given that any given API client will be acting on behalf of a particular user [09:51] rogpeppe, and fwiw that facade is really the api server, anyway, isn't it? [09:51] fwereade: sure, but we're about to change many many calls into state.State. [09:51] rogpeppe, expand a little maybe? ISTM that it's going to be even harder to do that [09:52] fwereade: type StateWithOwner {*state.State; User string} [09:53] fwereade: then StateWithOwner.AddService calls State.AddServiceOwnedBy [09:53] rogpeppe, so we s/State/StateWithOwner/ everywhere? [09:54] rogpeppe, not 100% sure that wins us anything [09:54] fwereade: i'm not sure either; worth thinking through though, i think. [09:55] rogpeppe, ISTM that the only benefit of what you propose is that we need to make fewer test changes [09:55] rogpeppe, but that each of those tests gets a bit more abstracted from what it's testing [09:55] fwereade: test changes are a big cause of churn and time taken [09:57] rogpeppe, I think that's an argument for a better way to define test fixtures, not an argument for messing with the code itself [09:57] fwereade: there are 231 calls to AddService [09:57] rogpeppe, how many places is AddService really used? once? twice? [09:58] fwereade: once [09:58] :-) [09:58] rogpeppe, ensuckening the code to accommodate shitty tests is not a win IMO [10:00] rogpeppe, I think the actual answer is in an AddService params struct [10:00] jam: HEY [10:00] er, caps [10:00] hey mgz [10:01] mumble? [10:01] rogpeppe, because I don't think this will be the only change like this [10:01] rogpeppe, eg it's crazy that we create a service and set its config in different txns [10:02] mattyw, ^^ - since you have to hit them all anyway, a params struct might not be a bad idea [10:02] fwereade, yeah just thinking about that, most of the changes seem to be in tests that don't need to know about an owner, this would be one way of getting around that [10:06] fwereade: a params struct seems reasonable [10:06] mattyw, mmmmm I worry a little that we actually *do* want an owner on every service [10:06] mattyw, the other possibility is to replace all of those with a helper in state/testing that adds a service with an implicit user-admin owner [10:06] fwereade, but in the tests where we don't need to test against an owner it seems odd to pass one in [10:07] fwereade: i'm thinking along those lines [10:07] fwereade: i'm wondering about something like testing.Deploy [10:07] fwereade: which would do the AddTestingCharm, add some number of units and return the results. [10:08] fwereade: which would hopefully match reasonably well to the requirements of lots of tests [10:08] fwereade: you pushed the ec2 and openstack Prepare() in. shall i continue now with the others? [10:08] mattyw, rogpeppe: I'd really not like to use the Deploy terminology anywhere we don't have to, it's pretty much crack [10:09] TheMue, that would be awesome, yes please [10:09] fwereade: really? [10:09] fwereade: ok [10:09] rogpeppe, yeah -- AddService and AddUnit are sane, promoting AddService+AddUnits to a primitive of its own is just silly at the model level [10:10] rogpeppe, fine, it's nice at the UI level [10:10] fwereade: istm that it's one of the best know juju commands and has reasonably well known semantics, so when we see it in a test we know what's happening [10:10] rogpeppe, but the command itself is about the user experience, not the underlying model [10:11] rogpeppe, when we're testing the model I would prefer that we not mix in concepts from higher levels [10:11] fwereade: i'm interested in making our test code smaller here [10:11] fwereade: how many testing AddService calls are *not* followed by an AddUnit ? [10:12] fwereade: and how many are not preceded by an AddTestingCharm call? [10:13] rogpeppe, the charm case is somewhat stronger than the unit one, I think [10:14] fwereade: i sometimes toy with the idea of allowing tests to set up stuff with some kind of textual DSL [10:14] rogpeppe, and I would prefer that mattyw implement a small step in a helpful direction than that he get lumbered with the burden of analysing and fixing additional context 230 tims [10:14] fwereade: sure, sorry, i've diverted here [10:14] rogpeppe, something like that would be great, I agree [10:17] axw: how have you been reproducing lp#1235717 ? [10:17] <_mup_> Bug #1235717: null provider bootstrap fails with error about sftp scheme [10:18] fwereade, rogpeppe sorry - internet went down [10:19] mattyw, no worries [10:19] (maybe not the whole thing - but certainly my view of it) [10:19] mattyw: np - you only missed my ramblin [10:19] g [10:19] mattyw: i think the conclusion is: add a params struct [10:19] rogpeppe, mattyw: wait a mo [10:20] rogpeppe, mattyw: a params struct is, independently a somewhat good idea, but I think I'd rather see a trivial AddService helper in state/testing, that takes the same args and just sets user-admin [10:21] fwereade: seems reasonable - then we can make the call itself panic if it gets an empty owner, perhaps [10:21] rogpeppe, mattyw: I wouldn't say no to a params struct as well ofc [10:21] rogpeppe, yeah, something like that [10:21] rogpeppe, mattyw: we *will* need to make sure juju still works on services with empty owner fields [10:22] rogpeppe, mattyw: but the StateSuite(?) has references to all those collections anyway, so we can mess them up in the DB and check they still work [10:22] rogpeppe, mattyw: however once service owners are part of the model I think it would be foolish to allow them to remain unset in normal use [10:23] rogpeppe, matty: probably not a panic though, this stuff will be happening in the api server [10:23] fwereade: unless, for backward compatibility purposes, we say that blank means admin [10:23] rogpeppe, internally we do [10:23] rogpeppe, no excuse for muddying up the interface [10:24] fwereade: agree [10:27] fwereade, rogpeppe ok, just want to make sure I understand this right: First step is to make a helper in state/testing for addservice, this will call the addservice function as is but also set the admin user inside the service? [10:27] mattyw: i think so, yes [10:28] and all the calls to addservice in the tests should use this new helper? or only the ones that need the owner set? [10:34] mattyw: all the calls, i think [10:35] fwereade: looking at the null provider, it's a pity that "null" needs to be quoted in environments.yaml. i wonder if we should've gone with the "nil" provider instead... [10:39] rogpeppe: sorry, was afk [10:39] yes, I have reproduced [10:39] fwereade: do you think we should print admin-secret in the output of juju init? [10:39] rogpeppe, ha, we should be dropping those really, shouldn't we [10:40] axw: i wanted to reproduce for myself - was wondering how you did [10:40] 1. bootstrap. 2. stop juju processes; remove /etc/init/juju*, everything in /var/lib/juju *except* storage/tools/releases/*; bootstrap again [10:40] fwereade: well, it's potentially useful [10:40] rogpeppe, who for?> [10:40] fwereade: but then again there are probably other sources for info on provider settings [10:40] rogpeppe: going to make/have dinner, I'll write down the steps properly a bit later [10:40] fwereade: anyone that wants to have a known admin secret login [10:40] axw: thanks [10:42] fwereade: for example if i want to use the web GUI interface on one of my environments currently, i have to type in something like c1c4cb509044b524b35585f0fb259646 which isn't ideal [10:44] rogpeppe, pasting it in isn't so hard though :) [10:45] dimitern: TheMue: standup? [10:45] https://plus.google.com/hangouts/_/calendar/am9obi5tZWluZWxAY2Fub25pY2FsLmNvbQ.mf0d8r5pfb44m16v9b2n5i29ig [11:02] rogpeppe, fwereade does the helper want to go in state/testing or in juju/testing in the JujuConnSuite (alongside addtestingcharm for example) [11:03] mattyw: the latter [11:03] rogpeppe, make more sense, ok thanks [11:07] fwereade, rogpeppe: https://codereview.appspot.com/14486043 [11:15] * TheMue => lunch [11:16] so, are the dev releases of juju released to the public or are they internal only? [11:16] natefinch: dev releases are public [11:17] natefinch: 1.15.1 is in the public s3 bucket, the hp storage container, tec. [11:17] etc [11:17] jam: interesting.... I haven't built a windows installer for it, so I'm assuming the installer on the website is still 1.14 [11:18] natefinch, that's still the "stable" release [11:19] dimitern: ahh, ok, so the dev releases are public, but don't replace the latest stable release [11:19] natefinch, I think so yeah [11:20] dimitern: basically wanted to make sure I wasn't falling down on the job by not creating an installer for 1.15. [11:20] natefinch, I think there still should be an installer for every release, even dev ones [11:20] jam, fwereade ? ^^ [11:20] dimitern: that would seem to make sense. [11:27] natefinch: we would still want a 1.15 installer, otherwise we can't test that it is suitable for being a stable release. [11:28] ideally it would just be part of the release process [11:28] but the need for a Windows machine makes it slightly tricky [11:28] natefinch: if you could easily set it up with an automated "ec2" sort of script, then someone like curtis could kick it off after cutting the tarball [11:31] jam: You mean a script to create the installer? I certainly could, but I don't know about "easily" :) It's sort of an 8 step process if you 're starting from a bare machine and need to build both the juju client and the installer from it. [11:31] natefinch: so the idea would be you *could* do a Windows ec2 instance so that anyone on the team could have access to it. [11:31] It would be possible to bundle the final image [11:31] so you don't have to do all the setup yet-again [11:32] I used that in the past for other projects and it was ~ok, not great, but reasonable [11:32] jam: ahh, yeah that would certainly be easier than scripting the install of Go and all that :) [11:34] jam: dang, I was thinking I should just write a charm to do it... but of course that won't work on windows :) [11:43] hmm, [11:44] axw: one problem is: tools.WriteMetadata actually ignores any metadata that it finds that corresponds with tools already in the target storage [11:45] axw: if we fixed that, the problem would disappear for almost all the time [11:45] axw: and make sync tools faster too [11:50] rogpeppe, fwereade, review poke [11:51] dimitern: sorry, looking [12:06] rogpeppe: hmm yeah, I guess it should check if size/sha256 are initialised or not, and use that to decide to blat over the top [12:07] axw: it only appends to toolsList [12:07] axw, size/sha256 are never initialized if you upgrade from 1.14 btw [12:08] axw: whereas it actually want to update it if there are some entries in existingMetadata that are also in toolsList already [12:08] s/want/wants/ [12:09] axw: "Merge in existing records" is a bit of an overstatement [12:12] axw: i wonder if things might work fine if we just ignored the List entirely. [12:12] axw: and just merged the metadata [12:13] rogpeppe: I don't really get it. In the scenario I've been investigating, len(existingMetadata) == 0 [12:13] axw: really? interesting. [12:14] rogpeppe: i.e. the tools tarballs are in storage, but no the metadata json [12:14] not* [12:14] axw: i tried a scenario where there *is* existing metadata, but we still have the same problem [12:14] ah [12:14] rogpeppe: can you please describe it? [12:15] axw: i bootstrapped with the null provider (to localhost) [12:15] axw: (the bootstrap failed because i'm already running the local provider, but it doesn't matter in this case) [12:15] axw: then i tried bootstrapping again, and saw the "sftp:" problem [12:16] axw: and i can verify that the metadata is all there [12:16] ah ok [12:16] rogpeppe: that's probably a more plausible explanation for what Bjorn reported :) [12:17] axw: and looking at WriteMetadata, i can't see how any hash/size metadata for existing tools in the target storage would get into the metadata without re-reading them [12:18] axw: because the toolsList argument won't have that metadata and the "Merge in existing records" loop doesn't change any entries in that argument [12:18] axw: yes - it'll happen every time [12:18] rogpeppe: ahh, because it's appending, not replacing... [12:18] axw: so i'm wondering whether a better approach is just to ignore tools in the target bucket if they don't have metadata [12:19] axw: yes [12:19] ok [12:19] I understand now, thanks [12:19] axw: it appends only if it doesn't find any existing entries [12:20] axw: the only down side i can currently see about ignoring tools with no metadata is that we may redo work, re-copying tools that were already copied [12:20] axw: which will happen if the upload process was aborted half way through [12:23] g'morning (relatively) i think the recent simplestreams/tools rejigger broke what used to be a correct stackoverflow answer that's still being referenced/used, if anyone has a minute it would be worth correcting http://askubuntu.com/questions/327177/cannot-bootstrap-due-to-precise-images-in-regionone-with-arches-amd64-i386 [12:29] dimitern: reviewed [12:29] rogpeppe, cheers [12:29] fwereade, poke [12:31] rogpeppe, I wouldn't mix tags into state - they're an api concept [12:40] rogpeppe: seems to me that SyncTools should be calling WriteMetadata with only "missing", not targetTools+missing [12:41] and whatever's where already should be assumed to be correct already [12:41] s/where/there/ [12:41] i.e. only update metadata for things that are copied === gary_poster|away is now known as gary_poster [12:45] dimitern: Tag is implemented on State objects [12:45] rogpeppe, but not used otherwise for queries and such [12:46] dimitern: does that matter here? this an error message for users [12:46] dimitern: i guess users don't really see tags either though [12:48] rogpeppe, well [12:48] rogpeppe, yeah I suppose it'll reduce the code a bit [12:49] dimitern: yeah, i think just a simple list of agents would work just as well [12:50] axw: i've been wondering about the "abort half way through problem" and perhaps this is a stupid idea, but what if we took the metadata as primary, and always copied that *first*? [12:51] axw: and avoid the use of Storage.List entirely [12:51] axw: well, we couldn't avoid it entirely [12:51] axw: but we'd only use it to find tools that the metadata mentions that don't show up in the list [12:51] * dimitern => lunch [12:52] axw: if we do things that way, the first thing SyncTools would do would be to merge the metadata [12:53] axw: and then it would copy across the actual tools data as necessary [12:53] rogpeppe: if it then fails while copying tools, we're going to have a bad time, no? [12:54] next time we do a sync [12:54] it'll think the target has everything [12:54] axw: no, because we make sync actually check the List [12:57] axw: at least, i *think* it could work ok [12:57] axw: it'll potentially be a problem if we *haven't* done the sync [13:00] rogpeppe: it sounds viable to me. I'd probably want to talk to wallyworld before doing it tho [13:01] axw: for the time being, i think we should just ignore the target tools list [13:01] axw: i'm really not very keen on the localhost storage wrapper solution [13:02] rogpeppe: I'm not keen on the fact that URL() is there at all [13:02] axw: it's there because we need it [13:03] axw: it's our way of telling a remote cloud-init shell script how to get the juju tools [13:03] rogpeppe: yeah I know the general use case, but I mean, it doesn't make sense for sshstorage [13:03] since it's transient [13:03] axw: perhaps you could make the method return an error? [13:04] so I suppose you're right [13:04] yeah [13:04] it should return an error, and we should change this stuff to never call it [13:04] axw: yeah [13:06] fwereade: do you know if there's still a case where we do any kind of "best-fit" matching to the available tools? even juju bootstrap now picks an exact match, no? [13:08] rogpeppe: by "ignore the target tools list", do you mean only update metadata for copied tools? [13:09] axw: i mean that we make our copy decisions based on the source and target metadata only [13:09] s/we make/we should make/ [13:10] axw: that way we ensure that we always have the hash and size metadata === gary_poster is now known as gary_poster|away [13:15]