| thumper | o/ axw | 01:41 |
|---|---|---|
| axw | hey thumper | 01:42 |
| thumper | axw: can I talk some things through with you? | 01:42 |
| axw | thumper: certainly | 01:42 |
| axw | hangout? | 01:42 |
| thumper | axw: https://plus.google.com/hangouts/_/72cpidoufej0otv9gnrrc6cbns?hl=en | 01:42 |
| axw | thumper: are you aware of any particular reason why we use a static bootstrap nonce? | 03:38 |
| thumper | axw: nope | 03:38 |
| axw | thumper: I'm thinking of using that to verify that the machine is in fact the bootstrap node | 03:38 |
| axw | would need to be made a UUID tho | 03:38 |
| thumper | I can't really think of a reason not to change the bootstrap nonce to a uuid | 03:39 |
| thumper | I would consider checking with fwereade just in case there is a reason we aren't aware of | 03:39 |
| axw | thumper: thanks, will do | 03:39 |
| thumper | axw: how to I assert in a test that two slices may have the same content but have different underlying arrays? | 03:53 |
| thumper | axw: or in another way, should I care? | 03:53 |
| axw | thumper: DeepEquals | 03:53 |
| axw | oh | 03:53 |
| thumper | axw: I'm writing a gnuflag.Value for []string | 03:54 |
| axw | you want to know that they *must* be different? | 03:54 |
| axw | why? | 03:54 |
| thumper | that wrapts the comma separated values | 03:54 |
| thumper | I guess I shouldn't care | 03:54 |
| thumper | the default value is fine? | 03:54 |
| thumper | value[:] forces a copy? | 03:54 |
| axw | [:] creates a new slice, but the backing array/slice is the same | 03:55 |
| thumper | I think I'm caring too deeply | 03:55 |
| thumper | I think this'll be ok | 03:55 |
| * thumper discards the need to be different | 03:55 | |
| axw | sounds like it, but I don't quite understand the use case :) | 03:55 |
| thumper | axw: I'll show you the code in a while :) | 03:57 |
| axw | okey dokey | 03:57 |
| thumper | axw: http://paste.ubuntu.com/6581765/ | 04:15 |
| axw | looking | 04:15 |
| thumper | axw: and the tests http://paste.ubuntu.com/6581766/ | 04:15 |
| thumper | seems to work how I'd like | 04:16 |
| thumper | also noticed that gnuflag already has a time.Duration type | 04:16 |
| thumper | which uses the go ParseDuration | 04:17 |
| axw | cool | 04:17 |
| thumper | A duration string is a possibly signed sequence of decimal numbers, each with optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". | 04:17 |
| thumper | but no days | 04:17 |
| thumper | but I think that's fine | 04:17 |
| thumper | not quite sure what -1.5 hours is | 04:17 |
| thumper | but can set the test timeout to 50ms :) | 04:18 |
| axw | yeah I think that'll do | 04:18 |
| axw | thumper: where were you going to check backing arrays? | 04:18 |
| axw | looks fine as is anyway | 04:21 |
| thumper | axw: probably paranoia | 04:37 |
| thumper | axw: but I agree, seems fien | 04:37 |
| thumper | fine | 04:37 |
| thumper | axw: is there a pop front type method on slices? | 04:41 |
| axw | thumper: nope | 04:41 |
| thumper | :( | 04:41 |
| axw | [1:] | 04:41 |
| thumper | except I need to do: value, args := args[0], args[1:] | 04:42 |
| thumper | when I want to write: value = args.pop_front() | 04:42 |
| * thumper shrugs | 04:42 | |
| thumper | see y'all tomorrow | 05:17 |
| dimitern | guys, fwereade has power issues and is waiting for someone to come and fix them - just texted me, so i'm letting y'all know | 07:12 |
| axw | thanks dimitern (and hi) | 07:29 |
| axw | dimitern, jam: do either of you know if there's a reason why we have a static nonce for the bootstrap node? | 07:29 |
| dimitern | hi axw | 07:30 |
| dimitern | axw, yes, because there's no state yet to verify the nonce from | 07:30 |
| axw | dimitern: ok. it won't break anything if I make it random tho right? I see very few references to it | 07:31 |
| axw | dimitern: I'd like to use it to verify the machine connected to during synchronous bootstrap is the right one | 07:31 |
| dimitern | axw, in some places it might be specified as a literal, rather than a const | 07:31 |
| axw | ah yeah, didn't check that | 07:31 |
| axw | nah, just a test. I think it's safe | 07:32 |
| dimitern | axw, as long as it can be verified at machine agent startup time (where CheckProvisioned is called), I see no problem with making it random, following the same scheme as for the others ofc | 07:32 |
| dimitern | axw, the idea behind the nonce format is to specify the parent machine that created the machine, i.e. "machine-0:<random id in hex>" | 07:34 |
| dimitern | axw, since the bootstrap machine has no parent, it might be "user-admin:<random id>" perhaps? | 07:34 |
| axw | dimitern: yeah that sounds sensible, I guess that was the rationale behind what it is now | 07:35 |
| dimitern | axw, yes, precisely | 07:36 |
| axw | dimitern: thanks, I'll look into doing that. looks fairly trivial | 07:37 |
| jam | rogpeppe1: ping | 09:11 |
| rogpeppe1 | jam: pong | 09:11 |
| rogpeppe1 | jam: oh! | 09:11 |
| rogpeppe1 | jam: hangout, i guess! | 09:11 |
| rogpeppe1 | natefinch, dimitern, fwereade, jam, axw: review appreciated https://codereview.appspot.com/42760043 | 11:49 |
| rogpeppe1 | axw: this may well be directly useful for stuff you've been doing recently | 11:49 |
| natefinch | rogpeppe1, looking | 11:50 |
| rogpeppe1 | natefinch: thanks | 11:54 |
| natefinch | rogpeppe1, would reporting the last error by default, rather than the first error maybe be more useful? It would represent the most recent reason for failing. Reporting the first error could result in spurious information, such as when you're waiting for a network to come up, maybe the first few errors are "can't connect" but then the network is up and you get a 404 from the remote server... the 404 is a lot more useful | 12:34 |
| natefinch | to know. Sure, you can customize the moreImportant function, but seems like last error would generally be more useful. | 12:34 |
| rogpeppe1 | natefinch: hmm, possibly | 12:35 |
| rogpeppe1 | natefinch: i'm not sure though - the usual case will be that we're trying several addresses with fallbacks, rather than several attempts for one address. | 12:36 |
| rogpeppe1 | natefinch: is the error from the last address we try likely to be any more useful than the first one? | 12:37 |
| natefinch | rogpeppe1, maybe a more generally useful implementation for moreImportant would be something that returns an error instead of returning a boolean, that way if someone wants to just aggregate all the errors, they can add them all to a custom MultiError struct | 12:37 |
| rogpeppe1 | natefinch: possibly. | 12:38 |
| rogpeppe1 | natefinch: that would be easy enough to add if we decide we want that | 12:38 |
| rogpeppe1 | natefinch: i've gone with moreImportant for the time being because we already use that concept elsewhere in the system | 12:39 |
| natefinch | rogpeppe1, it's tricky.... returning just one error out of a N that are tried doesn't seem terribly useful, unless you're relatively sure they're all going to fail for the same reason. | 12:39 |
| rogpeppe1 | natefinch: agreed, it's not easy | 12:39 |
| natefinch | rogpeppe1, I guess if they want to aggregate the errors, they can still do that with your moreImportant function, they'd just store them outside | 12:39 |
| rogpeppe1 | natefinch: likewise though, it's not great to print lots of similar errors to the user | 12:40 |
| natefinch | rogpeppe1, absolutely | 12:40 |
| natefinch | rogpeppe1, man, I hate the name tomb. | 12:51 |
| rogpeppe1 | natefinch: for its morbid associations? | 12:52 |
| natefinch | rogpeppe1, nah, I don't care about that, it's just so uninformative, I forget what exactly it does until I go look at it. | 12:53 |
| rogpeppe1 | natefinch: interesting. i find it ok | 12:54 |
| natefinch | rogpeppe1, I'm sure once I've used it a bunch it'll be fine. but when I first see it, the name just doesn't tell me much. I guess I'd prefer something more boring and generic like gowatcher or something (one of the few times when go in the name of the package actually helps describe the package) | 12:56 |
| rogpeppe1 | natefinch: gowatcher wouldn't really be right | 12:56 |
| rogpeppe1 | natefinch: it really is a bit like a tombstone - it acts as a marker for a death | 12:57 |
| rogpeppe1 | natefinch: interestingly, tomb is another place that could do with a combineErrors/moreImportant function | 12:57 |
| natefinch | rogpeppe1, I guess my problem is that you write tomb.Dying() and tomb.Dead() and tombs are never either | 12:58 |
| natefinch | rogpeppe1, maybe redshirt would be a better name.... something that is just inevitably going to die, and the only question is when | 12:58 |
| rogpeppe1 | natefinch: yeah, it's just a signifier | 12:58 |
| rogpeppe1 | natefinch: redshirt?? | 12:58 |
| natefinch | rogpeppe1, star trek joke. In the original star trek, it's really common for the guys in the red shirts (generally no-name crew members) to die when the crew goes on missions to a planet etc | 12:59 |
| rogpeppe1 | natefinch: ah. i don't do star trek. | 13:00 |
| natefinch | rogpeppe1, a way for the TV show to say "look, it's dangerous!" without killing off anyone you care about | 13:00 |
| === gary_poster|away is now known as gary_poster | ||
| natefinch | rogpeppe1, I'm not a huge trekkie, though I did enjoy the shows when they were airing. | 13:01 |
| rogpeppe1 | natefinch: i like sf :-) | 13:01 |
| natefinch | rogpeppe1, I love sci-fi. no time for tv & movies with the young kids, but that'll just give me all the more stuff to catch up on when they're older :) | 13:02 |
| rogpeppe1 | natefinch: i don't consider star trek to be sf... :-) | 13:03 |
| natefinch | rogpeppe1, oh come on, just because it's dated and campy... it may not pass for *good* sci fi these days, but it's still sci fi :) | 13:05 |
| rogpeppe1 | natefinch: it certainly has sf trappings | 13:06 |
| natefinch | rogpeppe1, seems like Try.Wait() isn't really useful... it's the same as _, err := t.Result() | 13:18 |
| rogpeppe1 | natefinch: it's useful because it means that Try fits the normal Kill/Wait interface for workers | 13:18 |
| rogpeppe1 | natefinch: so you can use worker.Stop on it, for example | 13:19 |
| natefinch | rogpeppe1, Ahh, I didn't make the connection. | 13:20 |
| natefinch | rogpeppe1, why not call the package try? t := try.New() t.Start() etc | 13:23 |
| rogpeppe1 | natefinch: because i thought it fitted quite neatly into the parallel package | 13:23 |
| rogpeppe1 | natefinch: parallel.Try reads quite nicely, i think | 13:23 |
| natefinch | this seems like an excellent case for a package that should be outside juju-core | 13:25 |
| rogpeppe1 | natefinch: and it's about the same level of abstraction as parallel.Run | 13:25 |
| natefinch | rogpeppe1, I actually missed that parallels was already a package :) | 13:26 |
| rogpeppe1 | natefinch: perhaps so, but i'd prefer to keep in internal for the time being | 13:26 |
| rogpeppe1 | s/in int/it int/ | 13:26 |
| rogpeppe1 | natefinch: it's certainly a candidate for abstraction into an external package at some point | 13:26 |
| natefinch | rogpeppe1, I disagree, because "for the time being" pretty much always ends up being forever, but it's not really my call | 13:27 |
| rogpeppe1 | natefinch: please, i really don't want to spend more time on overhead currently. | 13:28 |
| rogpeppe1 | natefinch: and an external package is just overhead at this point. | 13:28 |
| natefinch | rogpeppe1, like I said, not my call :) I'll submit my comments. LGTM in general | 13:29 |
| rogpeppe1 | natefinch: thanks | 13:29 |
| rogpeppe1 | natefinch: i've just changed the code to use combineErrors rather than moreImportant, FWIW | 13:30 |
| natefinch | cool | 13:33 |
| axw | rogpeppe1: I won't be much chop, reviewing under the influence. does indeed look like it'll be useful for bootstrap though | 13:34 |
| rogpeppe1 | axw: reviewing under the influence is so much more fun :-) | 13:34 |
| axw | rogpeppe1: I did a reflect-based thing here https://codereview.appspot.com/40860048/patch/40001/50014 | 13:34 |
| axw | I did wonder if generalising would be useful | 13:35 |
| rogpeppe1 | axw: ah, i think i saw your comment there and remember thinking i was dubious about using reflect.Select | 13:35 |
| rogpeppe1 | axw: in general it's almost always a bad idea to use reflect.Select as a substitute for variable-length select in Go, IMHO | 13:36 |
| axw | rogpeppe1: yeah, I agreed. I will rework it tomorrow | 13:38 |
| axw | *agree | 13:38 |
| axw | using your code if it's landed :) | 13:38 |
| rogpeppe1 | axw: will be landing v soon :-) | 13:38 |
| axw | rogpeppe1: aside from that bit, I'd appreciate a review for general soundness of what I've done | 13:39 |
| rogpeppe1 | axw: will do | 13:40 |
| axw | rogpeppe1: any particular reason for not taking an error in Kill? | 13:50 |
| axw | in synchronous bootstrap I pass an error in on SIGINT | 13:50 |
| axw | then that comes out the other end and up the stack | 13:50 |
| rogpeppe1 | axw: it fits the usual Kill paradigm | 13:50 |
| rogpeppe1 | axw: (see the worker.Worker interface, for example) | 13:51 |
| rogpeppe1 | axw: if you want to distinguish between ways that you've killed it, you'd need to record that somewhere else | 13:51 |
| axw | rogpeppe1: ok | 13:51 |
| rogpeppe1 | axw: i'd probably have something waiting for either interrupt or timeout to happen - then you wouldn't need a mutex | 13:52 |
| rogpeppe1 | axw: that's an example of why it's nice to map signals to channels | 13:53 |
| axw | rogpeppe1: yeah, fair enough | 13:53 |
| axw | I haven't gotten back to the other one yet | 13:54 |
| axw | (refactoring BootstrapContext) | 13:54 |
| rogpeppe1 | axw: ah yes, i was just looking to see if i'd missed it somewhere | 13:54 |
| axw | rogpeppe1: I was hoping to get bootstrap actually working again before fixing that bit up | 13:55 |
| axw | maybe it'll be involved tho | 13:55 |
| === benji_ is now known as benji | ||
| dimitern | rogpeppe1, hey | 14:21 |
| rogpeppe1 | dimitern: hiya | 14:21 |
| dimitern | rogpeppe1, what user do you think should be used to upload charms through the cli? | 14:21 |
| rogpeppe1 | dimitern: we've only got one user currently, so not much choice :-) | 14:22 |
| dimitern | rogpeppe1, for now I hard coded user-admin + admin-secret | 14:22 |
| rogpeppe1 | dimitern: i think that's correct currently | 14:23 |
| dimitern | rogpeppe1, ok | 14:23 |
| rogpeppe1 | dimitern: it's the usual command line user | 14:23 |
| dimitern | rogpeppe1, too bad some tests are not written with that in mind | 14:23 |
| rogpeppe1 | dimitern: really? | 14:23 |
| dimitern | rogpeppe1, like the apiserver/client tests | 14:23 |
| === _mup__ is now known as _mup_ | ||
| rogpeppe1 | dimitern: how so? | 14:23 |
| dimitern | rogpeppe1, yeah, it adds user "admin" and sets some default password | 14:24 |
| dimitern | rogpeppe1, which simplifies the test a bit but wreaks havoc with putcharm | 14:24 |
| rogpeppe1 | dimitern: ah - that's actually correct | 14:24 |
| rogpeppe1 | dimitern: you shouldn't hardcode adminsecret | 14:24 |
| rogpeppe1 | dimitern: you can hardcode user-admin, but the password should be checked against state's admin password | 14:25 |
| dimitern | rogpeppe1, what do you mean? I'm getting it from Environ.Config().AdminSecret() | 14:25 |
| dimitern | rogpeppe1, if I get it from state we're not entirely divorcing the cli from state | 14:26 |
| rogpeppe1 | dimitern: state is where the user info is stored | 14:26 |
| rogpeppe1 | dimitern: you should be using state.User.PasswordValid to check the user | 14:27 |
| rogpeppe1 | dimitern: but i think that's already done for you, isn't it? | 14:27 |
| dimitern | rogpeppe1, so you think the Conn object will have access to state even after CLI API is done? | 14:27 |
| rogpeppe1 | dimitern: so all you need to do is check that user is user-admin | 14:27 |
| dimitern | rogpeppe1, I do that in the apiserver, yes | 14:27 |
| rogpeppe1 | dimitern: ah, sorry, i thought you were talking about the server side | 14:27 |
| rogpeppe1 | dimitern: in the client, yes, it'll need to use admin-secret, just like all the other parts of the CLI that talk to the API | 14:28 |
| dimitern | rogpeppe1, so that's what I'm doing, but that apiserver/client test with the setUpScenario monstrosity changes the admin user password | 14:31 |
| dimitern | rogpeppe1, without touching the environ config, and hence the problem | 14:32 |
| dimitern | rogpeppe1, I'm refactoring those tests not to rely on a "default password" for user-admin | 14:34 |
| rogpeppe1 | dimitern: one mo - how do the tests work currently? | 14:35 |
| rogpeppe1 | dimitern: i don't quite see why putcharm is any different from the other CLI tests | 14:35 |
| dimitern | rogpeppe1, look at api_test.go:setUpScenario | 14:35 |
| dimitern | rogpeppe1, one of the very first things we do is u, err := s.State.User("admin"); setDefaultPassword(c, u) | 14:36 |
| dimitern | rogpeppe1, which screws up AddTestingCharm(), which uses PutCharm() internally, relying on admin-secret from the environ config to be the user-admin's password | 14:37 |
| rogpeppe1 | dimitern: so how do the existing tests know how to log in using that password? | 14:37 |
| rogpeppe1 | dimitern: so AddTestingCharm is making its own connection to the state? | 14:37 |
| dimitern | rogpeppe1, they don't - all tests assume the password is all the same for all entities: tag + " password-1234567890" | 14:37 |
| rogpeppe1 | s/the state/the API/ | 14:38 |
| dimitern | rogpeppe1, no, ATC() uses Conn.PutCharm internally, and put charm now does a https-basic-authenticated upload request | 14:38 |
| rogpeppe1 | dimitern: does AddTestingCharm need to go through the API? | 14:39 |
| dimitern | rogpeppe1, it currently uses PutCharm | 14:40 |
| dimitern | rogpeppe1, do you suggest otherwise? | 14:41 |
| * rogpeppe1 thinks | 14:41 | |
| dimitern | rogpeppe1, the code for uploading, calculating the hash, updating state, etc. is all there | 14:41 |
| dimitern | rogpeppe1, I haven't changed that, I'm only trying to make it work | 14:42 |
| rogpeppe1 | dimitern: presumably AddTestingCharm now uses s.APIConn.PutCharm ? | 14:42 |
| dimitern | rogpeppe1, there's no such thing | 14:44 |
| rogpeppe1 | dimitern: hmm. i didn't realise juju.Conn ever talked to the API | 14:44 |
| dimitern | rogpeppe1, only PutCharm does | 14:45 |
| rogpeppe1 | dimitern: that seems a bit wrong to me | 14:45 |
| dimitern | rogpeppe1, with my changes, and only to the https handler for charm upload | 14:45 |
| rogpeppe1 | dimitern: currently a Conn is just Environ+state.State | 14:45 |
| rogpeppe1 | dimitern: there should be no API dependency there | 14:45 |
| dimitern | rogpeppe1, how about 1.16 compatibility? | 14:46 |
| rogpeppe1 | dimitern: the plan is to deprecate Conn entirely for user use eventually | 14:46 |
| rogpeppe1 | dimitern: *eventually* | 14:46 |
| dimitern | rogpeppe1, right now addCharm() is the internal method that does the packaging, hash calc + upload to storage using state, and it's called inside PutCharm | 14:46 |
| dimitern | rogpeppe1, what I did is to move that code to addCharm1dot16 and add a new code to upload the charm through the API | 14:47 |
| rogpeppe1 | dimitern: so, firstly, PutCharm should be defined inside APIConn, not Conn | 14:47 |
| dimitern | rogpeppe1, how can we have both 1.16 compatibility and API access? | 14:47 |
| rogpeppe1 | dimitern: we can leave Conn.PutCharm as is for the time being | 14:48 |
| dimitern | rogpeppe1, hmm, ok | 14:48 |
| dimitern | rogpeppe1, and once we're not using it inside the CLI, we can drop it (or put it inside a testing package only, for AddTestingCharm's sake) | 14:48 |
| rogpeppe1 | dimitern: yeah | 14:48 |
| dimitern | rogpeppe1, but all that won't solve the issue with how the client api tests are written | 14:49 |
| rogpeppe1 | dimitern: i think that's not too hard - we just need a version of AddTestingCharm that uses a given API connection. | 14:50 |
| dimitern | rogpeppe1, ok, I'll look into that, thanks | 14:50 |
| rogpeppe1 | dimitern: there's an interesting issue there actually | 14:51 |
| rogpeppe1 | dimitern: which is that we need to keep the API connection password lying around so that we can use it again | 14:51 |
| dimitern | rogpeppe1, expand please | 14:52 |
| rogpeppe1 | dimitern: so, when you open an API connection, you use a tag and password, right? | 14:52 |
| rogpeppe1 | dimitern: if you later use PutCharm on that connection, the API needs to authenticate for the PUT request | 14:53 |
| rogpeppe1 | dimitern: so it needs the password | 14:53 |
| rogpeppe1 | dimitern: but currently the client-side API connection doesn't store the password that was used for the initial authentication AFAIK | 14:53 |
| dimitern | rogpeppe1, yeah | 14:54 |
| rogpeppe1 | dimitern: so we'll need to do that. | 14:54 |
| dimitern | rogpeppe1, but putcharm is a totally different per-request auth | 14:54 |
| rogpeppe1 | dimitern: agreed, but from the API point of view, it'll just be a method on client.State, right? | 14:55 |
| rogpeppe1 | dimitern: client.Client, rather | 14:55 |
| rogpeppe1 | api.Client even! | 14:55 |
| dimitern | rogpeppe1, not really | 14:57 |
| rogpeppe1 | dimitern: no? | 14:57 |
| dimitern | rogpeppe1, it's not a method, it's a server handler accepting requests | 14:57 |
| dimitern | rogpeppe1, and PutCharm is the client-side interface to that handler | 14:58 |
| dimitern | rogpeppe1, on a APIConn | 14:58 |
| rogpeppe1 | dimitern: but what does the client-side interface look like? | 14:58 |
| rogpeppe1 | dimitern: in Go, that is | 14:58 |
| rogpeppe1 | dimitern: i think it should be a method on api.Client | 14:58 |
| dimitern | rogpeppe1, it looks exactly like PutCharm, minus the bumpRevision flag | 14:58 |
| rogpeppe1 | dimitern: i don't see a reason for it to be on APIConn | 14:58 |
| rogpeppe1 | dimitern: in fact, i think APIConn will probably go entirely in time | 14:59 |
| dimitern | rogpeppe1, I don't see a reason for it to be on api.Client | 14:59 |
| dimitern | rogpeppe1, it's faking it if we put it there | 14:59 |
| rogpeppe1 | dimitern: what's the reason for APIConn to exist? | 14:59 |
| dimitern | rogpeppe1, you don't need an api connection to upload a charm | 14:59 |
| dimitern | rogpeppe1, api connection authenticated through login i mean | 14:59 |
| rogpeppe1 | dimitern: strictly speaking that's true, but in practice i think we're always going to want an API connection (to create the service), and if we do it this way, then it makes it easy for us to change to using API-obtained auth tokens in the future | 15:01 |
| rogpeppe1 | dimitern: which is something we may well want to do | 15:01 |
| dimitern | rogpeppe1, to use the API, but before charm upload support the only way to use the api was through an apiconn's web socket | 15:01 |
| dimitern | rogpeppe1, deploy for example does PutCharm before trying to do ServiceDeploy | 15:02 |
| dimitern | rogpeppe1, and gets a connection (apiconn) before that | 15:02 |
| rogpeppe1 | dimitern: sure, but it won't slow things down in the usual case if it does an API connection before doing the PutCharm, will it? | 15:02 |
| rogpeppe1 | dimitern: so it's making an API connection before doing the PutCharm anyway | 15:03 |
| dimitern | rogpeppe1, so what do you propose? caching the user/pass in api.client() after login? | 15:03 |
| rogpeppe1 | dimitern: yes | 15:04 |
| dimitern | rogpeppe1, I'll have to think about it | 15:04 |
| dimitern | rogpeppe1, it changes my whole approach | 15:04 |
| rogpeppe1 | dimitern: so if/when we decide in the future to use an auth token rather than user/pass, we can make the PutCharm method do that transparently | 15:05 |
| dimitern | rogpeppe1, most of the code in putcharm i could keep, but move it away from conn and leave the old one untouched | 15:05 |
| rogpeppe1 | dimitern: the only reason that juju.APIConn is around is for the pairing of Environ and api.Conn | 15:06 |
| dimitern | rogpeppe1, auth token or not, the deal doesn't change for putcharm - with cached auth creds | 15:06 |
| rogpeppe1 | dimitern: but we want clients to be able to use the API without an Environ | 15:06 |
| rogpeppe1 | dimitern: yeah | 15:06 |
| rogpeppe1 | dimitern: i don't think it changes too much | 15:06 |
| rogpeppe1 | dimitern: even if we don't make PutCharm a method on api.Client, it should still be implemented as a function inside the state/api package | 15:08 |
| dimitern | rogpeppe1, ok, will take that into consideration | 15:09 |
| rogpeppe1 | dimitern: thanks | 15:09 |
| dimitern | rogpeppe1, but unfortunately I won't propose it today as I hoped, due to the refactoring | 15:09 |
| rogpeppe1 | dimitern: sorry about that | 15:09 |
| rogpeppe1 | dimitern: FWIW the way i look at it is this: the state/api package abstracts all details of transport away from the client code, and that includes whether an individual request is made with a json RPC or an http PUT request | 15:10 |
| dimitern | rogpeppe1, it's ok, i appreciate the discussion - it opened up some things i didn't think about | 15:10 |
| rogpeppe1 | or POST request, even | 15:10 |
| rogpeppe1 | dimitern: cool | 15:11 |
| rogpeppe1 | dimitern: it also made me think about some issues i hadn't previously considered | 15:11 |
| dimitern | rogpeppe1, yeah, the time is not lost :) | 15:11 |
| rogpeppe1 | dimitern: i always find it interesting how a seemingly minor issue encountered just trying to make some tests pass can blow up into a significant design change | 15:12 |
| dimitern | rogpeppe1, :D it happens to me all the time | 15:14 |
| === _mup__ is now known as _mup_ | ||
| === jamespag` is now known as jamespage_realme | ||
| rogpeppe1 | natefinch: i've updated the CL, with one significant logic change | 15:38 |
| rogpeppe1 | natefinch: https://codereview.appspot.com/42760043 | 15:38 |
| * dimitern signs off, will be back later | 15:48 | |
| === makyo_ is now known as Makyo | ||
| natefinch | sometimes IRC is wacky | 15:54 |
| === gary_pos` is now known as garyposter | ||
| natefinch | rogpeppe1, dimitern: did either of you guys get a chance to run the tests on my branch? lp:~natefinch/juju-core/mongo-ha | 15:57 |
| rogpeppe1 | natefinch: ah, will do | 15:57 |
| === garyposter is now known as gary_poster | ||
| rogpeppe1 | natefinch: BTW in case you didn't see, i updated the Try CL | 15:58 |
| rogpeppe1 | natefinch: given your LGTM, i assumed you'd be ok with the changes, and have approved it, but i'd appreciate another once-over | 15:58 |
| natefinch | rogpeppe1, I'll double check, and thanks | 15:58 |
| natefinch | rogpeppe1, the failing tests are in environs/replicaset | 15:59 |
| rogpeppe1 | natefinch: i seem to have mislaid your branch link | 16:05 |
| * rogpeppe1 wishes the G+ chats weren't so ephemeral | 16:06 | |
| === hazmat` is now known as hazmat | ||
| rogpeppe1 | has anyone got access to the bot? it seems to have run out of temporary directories again | 16:19 |
| rogpeppe1 | fwereade: ^ | 16:22 |
| rogpeppe1 | fwereade: i don't think any branch can land currently | 16:22 |
| fwereade | rogpeppe1, um, not immediately, let me poke around | 16:23 |
| === Daviey_ is now known as Daviey | ||
| === jamespag` is now known as jamespage | ||
| === FunnyLookinHat__ is now known as FunnyLookinHat | ||
| rogpeppe1 | natefinch: i changed juju.NewAPIConn to use parallel.Try: https://codereview.appspot.com/37650048 | 18:17 |
| rogpeppe1 | natefinch: any update on the 'bot? | 18:17 |
| rogpeppe1 | fwereade: ^ | 18:17 |
| * rogpeppe1 is done for the day | 18:29 | |
| rogpeppe1 | natefinch: review of the above appreciated, if you have a mo. https://codereview.appspot.com/37650048 | 18:29 |
| rogpeppe1 | g'night all | 18:29 |
| rogpeppe1 | fwereade: ^ | 18:29 |
| natefinch | rogpeppe1, looking | 18:29 |
| natefinch | rogpeppe1, did you get a chance to run those tests? | 18:30 |
| thumper | morning juju hackers | 18:33 |
| natefinch | thumper, morning... you up early today? | 18:44 |
| thumper | natefinch: yeah | 18:51 |
| thumper | natefinch: school holidays and Rachel went to work early | 18:51 |
| thumper | so I thought I might as well get up and work | 18:51 |
| thumper | fwereade: if you are up and want a catch-up call, that would be good :-) | 19:00 |
| thumper | mramm: hey, hope you are feeling better | 20:02 |
| mramm | yep | 20:04 |
| mramm | not 100% but, quite a bit better | 20:05 |
| mramm | just got off the phone with hazmat | 20:05 |
| mramm | thumper: I see you in the hangout, but don't see you in the hangout ;) | 20:06 |
| thumper | hmm.. I'll rejoin | 20:06 |
| === hatch_ is now known as hatch | ||
| natefinch | thumper, what version of mongo do we officially support, server-side? I have a test failure in 2.2, but I'm pretty sure we only deploy 2.4 | 21:14 |
| thumper | I think we need 2.4 for the ssh bits | 21:15 |
| thumper | 2.2 doesn't have it | 21:15 |
| thumper | we are probably a bit shit in detecting this and enforcing minimum versions | 21:15 |
| natefinch | thumper, I think the bot has 2.2, which is why I asked... but I'm not sure of how to get to the bot to check | 21:26 |
| natefinch | thumper, not important now, I'm getting the test failure locally with 2.4 installed, so.... I'm back to square one, no easy out of "we don't support that anyway" ;) | 21:49 |
| thumper | heh | 21:50 |
| === gary_poster is now known as gary_poster|away | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!