| ion_ | :-) | 00:00 |
|---|---|---|
| Keybuk | wing-commander upstart% dbus-send --system --dest=com.ubuntu.Upstart --type=method_call --print-reply /com/ubuntu/Upstart/jobs/bar com.ubuntu.Upstart.Job.GetInstanceByName string:"debug" | 00:32 |
| Keybuk | method return sender=:1.1602 -> dest=:1.1605 reply_serial=2 object path "/com/ubuntu/Upstart/jobs/bar/debug" | 00:32 |
| Keybuk | kewlios | 00:32 |
| Keybuk | wing-commander upstart% dbus-send --system --dest=com.ubuntu.Upstart --type=method_call --print-reply /com/ubuntu/Upstart/jobs/foo com.ubuntu.Upstart.Job.GetInstanceByName string:"active" | 00:33 |
| Keybuk | Error com.ubuntu.Upstart.Error.Singleton: Singleton jobs do not have named instances | 00:33 |
| ion_ | I'm thinking that perhaps there should be no differentiation between singleton jobs and ones with multiple instances ‒ just assume 'instance default' or something similar as the default, which makes it a singleton and the same logic applies to all jobs. | 00:35 |
| Keybuk | the plan is for a GetInstance(Array of String env) function | 00:36 |
| Keybuk | which will hide the difference | 00:36 |
| ion_ | But does there need to be a difference in the first place? | 00:36 |
| Keybuk | it saves having a "magic" instance name | 00:37 |
| Keybuk | or other strange behaviours like all names are true | 00:37 |
| Keybuk | the difference is exposed to the job as well | 00:40 |
| Keybuk | singleton jobs don't get a $UPSTART_INSTANCE variable in their environment | 00:40 |
| Keybuk | nor do they export an $INSTANCE variable in their events | 00:41 |
| Keybuk | start on stopped apache failed | 00:41 |
| Keybuk | instead of | 00:41 |
| Keybuk | start on stopped apache MAGIC failed | 00:41 |
| Keybuk | it seemed like the right approach ;) | 00:42 |
| Keybuk | of course, it's more code to actually do it that way, so I'm willing to be persuaded ;-) | 00:42 |
| Keybuk | (after my bath - bbiab) | 00:42 |
| ion_ | I was thinking that it would be more elegant if the implementation under the hood didn't know the difference between singleton and instanced jobs and the user interface was the one that does something (didn't think that far yet) to avoid exposing the "magic" variable. It wouldn't really be magic, it would just be anything constant. | 00:47 |
| ion_ | But if the part about moving the separation from the job implementation to UI makes the UI code suck, it's not worth it of course. | 00:50 |
| ion_ | With UI, i mean the public API and job files. | 00:51 |
| ion_ | For instance, the API could expose a GetInstance method that only returns a single instance when the job's "instance" value does not reference a variable. | 00:54 |
| ion_ | And it would be kind of nice if 'start on stopped apache failed' caused the job to start whenever 0) any apache instance fails if apache has instances or 1) the apache instance fails if it's singleton. | 00:57 |
| ion_ | Perhaps have the instance after 'failed'? That wouldn't be as pretty though. | 01:00 |
| Keybuk | I originally put $INSTANCE at the end | 01:32 |
| Keybuk | but moved it forwards because it made more sense | 01:32 |
| Keybuk | start on stopped getty tty1 | 01:32 |
| Keybuk | start on stopped rc 2 | 01:32 |
| ion_ | Yeah | 01:33 |
| Keybuk | internally there isn't *much* difference | 01:33 |
| Keybuk | singleton jobs have a single instance called NULL | 01:33 |
| Keybuk | the "get instance" function always returns it | 01:33 |
| Keybuk | and things like job name in messages and environment know not to add the instance name | 01:34 |
| Keybuk | it is, admittedly, quite odd that | 01:40 |
| Keybuk | instance blah | 01:40 |
| Keybuk | and omitting instance have exactly the same effect | 01:40 |
| Keybuk | but behave differently | 01:40 |
| ion_ | It seems they decided that running water isn't needed here. | 01:41 |
| Keybuk | heh | 01:44 |
| Keybuk | badness | 01:44 |
| Amaranth | eh, who needs it? | 01:44 |
| Keybuk | maybe the right thing to do is accept "" as NULL | 01:44 |
| Keybuk | so GetInstanceByName("") would return it | 01:44 |
| ion_ | That sounds okay. | 01:45 |
| Keybuk | should it have UPSTART_INSTANCE="" in its environment and INSTANCE="" in its events? | 01:45 |
| Keybuk | ah, you can't _have_ empty environment variables | 01:46 |
| Keybuk | heh | 01:46 |
| ion_ | Hm | 01:46 |
| ion_ | % export foo=; env | grep foo | 01:46 |
| ion_ | foo= | 01:46 |
| ion_ | % sh | 01:46 |
| ion_ | $ env | grep foo | 01:46 |
| ion_ | foo= | 01:46 |
| Keybuk | $ foo= | 01:47 |
| Keybuk | $ $ env | grep foo | 01:47 |
| Keybuk | $ | 01:47 |
| Keybuk | oh, forgot export | 01:47 |
| Keybuk | damn | 01:47 |
| Keybuk | so back to the question | 01:47 |
| ion_ | Exactly what are the implications of having INSTANCE="" in the events? Still using the same parameter order for e.g. 'failed' events? | 01:48 |
| Keybuk | well, you'd have to do | 01:49 |
| Keybuk | start on stopped apache "" failed | 01:49 |
| Keybuk | or just use | 01:49 |
| Keybuk | start on stopped apache RESULT=failed | 01:49 |
| ion_ | Yeah... The latter one isn't that bad IMO. | 01:49 |
| Keybuk | is this nicer? | 01:50 |
| Keybuk | start on stopped apache FAILED=pre-start | 01:50 |
| Keybuk | or | 01:50 |
| Keybuk | start on stopped apache FAILED=* | 01:50 |
| Keybuk | ie. rather than have RESULT=failed /and/ PROCESS=pre-start, combine them | 01:50 |
| ion_ | I'm not quite sure which is nicer. | 01:51 |
| Keybuk | FAILED=* is a little less readable I guess | 01:51 |
| ion_ | True | 01:51 |
| Keybuk | and you can't invert match | 01:51 |
| Keybuk | ie you wouldn't be able to match _not_ failed ; | 01:51 |
| Keybuk | RESULT=ok works | 01:51 |
| ion_ | Right | 01:51 |
| ion_ | So, the default would be 'instance ""', where "" would be the canonical instance name for a singleton job? | 01:53 |
| Keybuk | yup | 01:54 |
| ion_ | Sounds good to me. | 01:55 |
| Keybuk | based on the above logic, that could be any string | 01:55 |
| Keybuk | is there something better than just "" ? | 01:55 |
| ion_ | The user would have the ability to do 'instance foo', but would just have to accept that then the canonical GetInstanceByName("") wouldn't work. | 01:55 |
| Keybuk | or should we leave the something better for people who want to "instance xxx" it for nefarious purposes | 01:55 |
| Keybuk | right, GetInstanceByName("foo") is what worked - they defined the instance name | 01:56 |
| ion_ | I find "" a good default. Just document that 'instance ""' is the right thing to do (and the default value) for a singleton job and GetInstanceByName("") is the canonical way to get to it. | 01:56 |
| Keybuk | and its dbus name would be /_ or something? | 01:57 |
| ion_ | Hm. Or perhaps have such magic in the d-bus code that GetInstanceByName("") and /_ point to the single instance whenever 'instance foo' doesn't refer to any variables? | 01:59 |
| ion_ | That would be the Official™ definition of singleton jobs: 'instance ...' doesn't refer to any variables. | 01:59 |
| Keybuk | that would be hardy | 01:59 |
| Keybuk | harder | 01:59 |
| Keybuk | freudian slip there | 01:59 |
| Keybuk | and I'm not sure that's right either | 01:59 |
| ion_ | Yeah... | 02:00 |
| Keybuk | I think it's quite valid to have a getty job with just "instance tty1" | 02:00 |
| Keybuk | you want to be compatible with someone else's getty job that does use instances | 02:00 |
| Keybuk | and you wouldn't want that working for tty2 | 02:00 |
| ion_ | How about 'instance $TTY' and fail if $TTY != "tty1" in pre-start script? | 02:01 |
| Keybuk | that's another way to do it | 02:01 |
| Keybuk | less clean though | 02:01 |
| Keybuk | I think that the instancebyname returning the answer for anything would be a little surprising | 02:02 |
| Keybuk | and leading people to actually check the name after to ensure what they had | 02:02 |
| Keybuk | e.g. | 02:02 |
| Keybuk | name = "tty4" | 02:02 |
| ion_ | True | 02:02 |
| Keybuk | instance = GetInstanceByName(name) | 02:02 |
| Keybuk | // can't assume instance->name == name | 02:02 |
| Keybuk | ok | 02:35 |
| Keybuk | as usual, 20s to do, 20m to update the tests | 02:35 |
| Keybuk | but now Upstart has _no_ singleton functionality | 02:35 |
| Keybuk | it just happens that the default value for instance is "" | 02:35 |
| ion_ | Nice | 02:36 |
| Keybuk | and yay | 02:46 |
| Keybuk | I got rid of the annoying function | 02:46 |
| ion_ | What was that? | 02:47 |
| Keybuk | job_instance () | 02:51 |
| Keybuk | it can now just be nih_hash_lookup | 02:51 |
| Keybuk | it was annoying because it really should have been inside job_class, but couldn't be, because it returned Job | 02:56 |
| Keybuk | wing-commander upstart% dbus-send --system --dest=com.ubuntu.Upstart --type=method_call --print-reply /com/ubuntu/Upstart/jobs/foo com.ubuntu.Upstart.Job.GetInstanceByName string:"" | 03:06 |
| Keybuk | method return sender=:1.1669 -> dest=:1.1671 reply_serial=2 object path "/com/ubuntu/Upstart/jobs/foo/_" | 03:06 |
| Keybuk | there we go | 03:06 |
| ion_ | Hmm | 13:00 |
| ion_ | keybuk: How about foo: 'instance $BAR', start foo BAR="" | 13:00 |
| ion_ | It would *work*, of course, but e.g. a GUI could not just test whether there's a running instance called "" and say it's singleton, it would have to check the value of 'instance ...' | 13:01 |
| Keybuk | why would the GUI care? | 13:13 |
| Keybuk | one of the things I like is that there's no such thing as a singleton job anymore | 13:14 |
| Keybuk | all jobs are multi-instance, just for different values of multi, including one | 13:14 |
| ion_ | Yeah, i didn't really think *why* the GUI should care. :-) But yeah, that's the elegance i meant. | 13:18 |
| sadmac2 | Keybuk: Fedora comes out on Tuesday | 13:56 |
| Keybuk | cool | 14:00 |
| Keybuk | man, D-Bus arrays are fiddly | 14:01 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!