[08:01] jam: hiya [08:08] hi rogpeppe1 === rogpeppe1 is now known as rogpeppe [08:10] jam: i was just thinking about your "single watcher for multiple agents" remark and wondering how you thought it might work [08:11] rogpeppe: on the client side, when you get an event from Changes it includes the context of what thing was changing. [08:11] jam: i'm talking about the watcher at the API level here, not the watcher on the state directly [08:11] jam: usually it doesn't [08:12] jam: e.g. the result from an EntityWatcher is struct{} [08:13] jam: it's true that if you want to watch multiple things, you'd probably need to interpose an intermediate goroutine to add the context [08:13] rogpeppe: right, I don't intend to use the EntityWatcher model, because the only thing we are watching is a state.Tools which is a small compact thing. It seems perverse to send an empty "something changed" rather than sending the actual 100 bytes of what changed. [08:13] jam: sure, that's fine [08:14] jam: what other watchers *do* include the context of what thing was changing? [08:15] rogpeppe: LifeCycleWatcher returns an array of things that changed, though I guess not what they changed to? [08:15] I mentioned it with fwereade__, and he seemed fine with it. [08:16] I can see for things you are worried about convergence, you want an event to go check there is something new. [08:16] jam: LifecycleWatcher doesn't tell you what collection of things you're watching (which is the equivalent) [08:16] jam: i think that sending the state.Tools is fine [08:16] jam: i don't think it's worth sending the Id though [08:17] jam: especially since i can't imagine a scenario where an agent is watching many things for tools changing, and even if it did, it's easy to work around. [08:18] jam: it's also easy to add the id at the client side if we want [08:19] jam: we already have an intermediate goroutine that calls Next and sends on the channel which has info about the id being watched [08:19] jam: it could easily add its local knowledge of the id to the channel without it needing to go across the network [08:29] jam: does that make sense? [08:30] rogpeppe: but why do you need N goroutines watching N watchers when you could have 1 goroutine watching 1 watcher watching N things? [08:31] jam: the upgrader is only watching one thing, right? [08:31] rogpeppe: the API is intended to allow you to watch more than one thing if you ask for it. [08:31] ATM it probably only watches one. [08:31] But why explicitly limit it [08:31] when it isn't that hard to have it do more [08:32] with the only "overhead" that the Change event includes slightly more context. [08:33] jam: ah, i see, you want the bulk call to return a single watcher for all the things being watched [08:33] rogpeppe: that was the idea, yes [08:38] jam: i see. that could work. except that it's probably more code (you have to keep track of more stuff at the server side, i think) and as with most of the bulk call stuff, i cannot think of any possible future scenario where we might possibly actually use this functionality. [08:41] jam: the whole point of this watcher is for a single entity to ask to be notified of its new tools. why would we ever want to watch multiple things? ok, i see (well i don't, but i go along with it) that for reasons of "habit", we make the initial Watch call a bulk call, but it's simpler, less code, easier to understand and just as general to return a watcher for each id (and in practice we'll only ever have one) than a single watcher that [08:41] watches many things. [08:42] jam: it's probably not even going to be measurably less efficient [08:42] rogpeppe: except if you ever want to transition to the case of more-than-one-thing you have to go into a fairly complex "create a bunch of goroutines adding context into a single channel", when a watcher that watches multiple things naturally handles all of that for you. [08:44] jam: that assumes that you have a single goroutine wanting to see changes on all those entities, of course. [08:45] jam: my visceral reaction is YAGNI here [08:48] jam: i think it will be much easier to write the server side if a Next call is looking up tools with respect to a single entity. [08:52] jam: hmm, maybe it's not too bad. here's a sketch of how Next might work at the server side in each case: http://paste.ubuntu.com/5791950/ [08:57] jam: you might want to have a single state watcher shared by the whole API server that watches the global version number. [08:58] jam: there are some interesting questions about the best way to send the info to all the watchers [09:04] jam: something like this might turn out to be useful: https://code.google.com/p/rog-go/source/browse/values/values.go [09:41] rogpeppe: what is wrong with my logic for AgentVersion ? [09:41] A user requests an upgraded version [09:41] that sets the global version [09:41] Machine agents notice [09:41] upgrade [09:41] when the upgrade finishes [09:41] they save their current actual version [09:41] Unit agents notice when their Machine agent versions upgrade [09:41] and they go to upgrade [09:43] You're right that if we want to do canary upgrades, we'd need a separate field. [09:43] Though we can make the API be "what version do you want me to be", and the first implementation just watches the global field. [09:44] And future versions can change that in a perfectly compatible manner. (I think) [09:45] jam: yes, that last seems good to me (but just thinking through the implications) [09:45] rogpeppe: some of this is more on the "lets make sure we can do what we think we need to" rather than "lets do this right now". [09:45] jam: +1 to that [09:47] jam: upgrades will be marginally slower (two poll intervals rather than one) but other than that, i can't think of any down sides. [09:49] jam: especially since every container is now a "machine", so we know that only one piece of code has the responsibility for actually downloading stuff. [11:12] rogpeppe: if you're still around, any idea why the last patch set of https://codereview.appspot.com/10437043/#ps2001 includes 'azure' changes? [11:12] jam: if you do a diff between two patch sets, it'll show you the actual diffs between the two branches [11:13] jam: and that has many changes because i'd merged trunk between the two [11:14] jam: but... [11:14] rogpeppe: sure, by why when you merged trunk didn't the updated diff be computed against trunk again? (I guess this is a difference between how LP computes diffs, and how lbox does) [11:14] jam: i don't see that happening here [11:14] jam: there's only one patch set, and i don't see any azure changes [11:14] rogpeppe: https://codereview.appspot.com/10437043/#ps5002 [11:14] (same CL, next patch set [11:15] Patch Set 2 looks quite sane to review. [11:15] Patch set 3 has lots of unrelated changes. [11:15] hmm, weird [11:16] jam: probably because i hadn't merge trunk to the prereq [11:16] merged [11:16] something like that sounds reasonable. [11:16] prereq + merge trunk can make lbox confused [11:16] Launchpad does prereq's differently than lbox [11:16] one mo, i'll sort it out [11:17] rogpeppe: you don't have to spend a lot of time. the "I merged trunk" is a sufficient answer. [11:17] I don't think you did much in your responses, I was just following along. [11:17] And didn't think you did lots of Azure changes in response to feedback :) [11:21] jam: i didn't. and i can't sort out the CL now [11:21] jam: "error: Failed to send patch set to codereview: diff is empty" [11:21] jam: unsurprisingly [11:21] rogpeppe: right, because your patch has landed [11:21] oh well [11:21] np [11:22] jam: if you're in reviewing mode, i'd appreciate your thoughts on https://codereview.appspot.com/10447047/ [11:22] jam: and also https://codereview.appspot.com/10259049/ (i know william's commented already, but the more eyes the better) [12:25] rogpeppe: golang question. How is it possible that net/fd.go defines a "netFD" struct, which is also defined by "fd_windows.go". [12:25] I think I understand the 'compile only modules that have _$GOOS' on the appropriate platform [12:26] does doing that override types that are defined in the root x.go file ? [12:26] jam: i don't see a file fd.go [12:27] jam: i only see fd_$GOOS.go [12:27] jam: for GOOS in {bsd, plan9, unix, windows} [12:28] rogpeppe: is that go-1.1 vs go 1.0.3? [12:28] because I clearly see src/pkg/net/fd.go [12:28] jam: perhaps; are you looking at go 1.0.3 [12:28] ? [12:28] jam: i'll check there [12:29] rogpeppe: right. And: grep -rnI "type netFD" returns 2 locations. One in 'fd.go' and one in 'fd_windows.go', but no definitions in fd_linux.go etc. [12:29] jam: ah, in 1.0.3, the build constraints on fd.go are: // +build darwin freebsd linux netbsd openbsd [12:29] jam: that doesn't include windows [12:29] rogpeppe: so the magic '+build' comment [12:29] jam: yeah, and it looks like x_$GOOS.go might be treated specially [12:30] jam: which i'd forgotten about - a bit of a possible gotcha [12:31] jam: ah yes: http://paste.ubuntu.com/5792362/ [12:31] jam: that's a good reason to avoid underscores in filenames unless using them for magic (e.g. _test.go) [12:31] jam: see go doc go/build for the lowdown [12:32] rogpeppe: sure, I knew about the _windows magic, as I've seen it quite a bit. I didn't know about +build [12:32] jam: ah! [12:32] jam: i knew about +build but had forgotten about the _windows magic :-) [12:32] I've probably seen more _amd64 magic. [12:32] jam: // +build ignore [12:33] jam: is quite useful sometimes [12:33] jam: for commenting out a file [12:33] rogpeppe: also, lbox doesn't compile on Windows because goetveld only has a 'terminal_linux.go' module, [12:37] jam: i don't see that [12:37] jam: i'm looking at revno 39 of goetveld [12:37] jam: which i just go got [12:40] jam: hmm, maybe i'm looking at a different tree. i can't see any terminal_linux.go in the revision history [12:41] rogpeppe: I have a version of goetveld that has a patch from Dave Cheney which renames terminal.go => terminal_linux.go and adds terminal_darwin.go [12:41] terminal.go itself doesn't compile on Windows [12:41] jam: ah [12:41] because it uses syscall.Termios functionality [12:41] that doesn't exist [12:42] jam: it would be easy enough to mock up a portable version of readPassword [12:42] jam: that didn't bother turning echo off [12:42] probably [12:42] jam: at least that would be a stand-by [12:42] it is trying to do direct-to-terminal with-no-echo which I understand is useful for passwords. [12:42] I tried to do something (because I was doing some juju work on Windows for enablement) [12:42] but I sort of gave up. [12:43] jam: yeah, it's useful but not essential [12:43] jam: you can always make sure there's noone else in the room :-) [12:43] jam: it should really take a *File rather than the fd [12:44] jam: the portable version could print a warning: "WARNING: your password will not be hidden when you type it" [12:45] rogpeppe: it looks like you might be able to use 'conio.h' and getch(): http://stackoverflow.com/questions/6856635/hide-password-input-on-terminal [12:45] though I don't really know how to get access to that without cgo. [12:45] Anyway, not my primary concern today :) [12:47] jam: it sounds like echo is off by default in windows [12:49] rogpeppe: I think 'getch' == characters with no echo, 'getc' is characters with echo from Posix. [12:49] rogpeppe: http://msdn.microsoft.com/en-us/library/078sfkak%28v=vs.80%29.aspx [12:51] jam: the os package has special code to invoke ReadConsole under windows. i wonder if you just called syscall.Read, it might work with no echo [12:54] jam: or this http://msdn.microsoft.com/en-us/library/windows/desktop/ms686033(v=vs.85).aspx [12:54] jam: but unfortunately syscall has GetConsoleMode but not SetConsoleMode [12:55] rogpeppe: I thought there was a non-cgo way of grabbing functions from kernel32.lib [12:55] jam: it's possible [12:55] jam: i haven't used go on windows at all [12:56] rogpeppe: at least, ISTR reading some other go code that didn't need cgo in order to call some windows API stuff. but maybe it just used the already exported syscall functions. [12:57] jam: yeah, i think you can probably use syscall.Syscall directly [12:58] jam: it's a pity that modkernel32 isn't exported, but perhaps it doesn't matter [13:01] rogpeppe: well, it looks like you could always directly call syscall.LoadDLL() if you really wanted. [13:01] jam: yeah, my reading of http://msdn.microsoft.com/en-us/library/windows/desktop/ms684175(v=vs.85).aspx is that it will be idempotent [13:08] " [13:08] something about "where is [13:08] this variable coming from" that I find hard to read with our current [13:08] idioms [13:08] " [13:08] jam: do you use emacs? [13:09] rogpeppe: vim [13:09] jam: ah [13:09] rogpeppe: I can use ctags + "^]" to go to a tag definition [13:09] jam: if you were using emacs, there's a plugin that uses my godef tool [13:09] it is still very 'somewhere else in this package you might find something' [13:09] jam: which is much much better than ctags [13:10] jam: it takes you *precisely* to the definition [13:10] jam: even if you click on "z" in the expression like f().x.y.z and z is defined as a field in many paces [13:10] places [13:11] jam: it might not be too much hassle to fit it into vim, though i'm not at all familiar with vim-as-programming-language [13:12] rogpeppe: there was a vim runnig-a-server-to-get-definitions program that I used for a while a year ago, but then I stopped doing go for a while and it isn't packaged so I haven't tracked it down again. [13:12] jam: http://godoc.org/code.google.com/p/rog-go/exp/cmd/godef [13:12] rogpeppe: https://github.com/nsf/gocode [13:13] That one does autocomplete on '.' sort of things. [13:13] jam: i think that did code completion but not definition finding AFAIR [13:13] jam: godef could theoretically be used to do code completion too [13:13] jam: as it knows about member names [13:14] jam: its dark side is that it's written against an ancient fork of the go parser [13:14] jam: some time i intend to make it use go/types, but "in my copious spare time" [13:14] jam: for the moment i use it constantly and it works in 99% of cases [13:15] jam: (and it's pretty instantaneous - runs in ~10ms) [13:17] rogpeppe: so that is your "rog-go/exp/go/ast' stuff? [13:17] jam: yeah [13:18] jam: it does symbol resolution at ast-construction time [13:18] jam: which is *almost* but not quite possible in general [13:19] jam: rather, it's not possible without reading all the imports, which i didn't want to do [13:19] jam: and that's made worse by import to . [13:19] jam: which godef doesn't support (probably its main limitation) [23:30] ah fark...