Hi Andre,
Hi Ceriel, [Cc to list]
I sent you the mail above about 3 weeks ago, did you get it?
*blush* I did get it, but it slipped down the stack. Sorry for that, and thanks for the reminder! Quoting [Ceriel Jacobs] (Sep 23 2010):
Ceriel Jacobs wrote:
Hi Andre,
I have yet again a couple of questions about the Saga specifications, this time about streams.
- the stream_server::serve() method has a timeout parameter, and the notes say: "if no client connects within the specified timeout, a 'Timeout' exception is thrown". This seems to contradict section 2.6.3, which sais: "These methods MUST not cause a Timeout exception as the timeout period passes, but MUST return silently". Should the serve() method indeed throw a Timeout exception? I guess serve() could just return null in this case.
I rather think that 2.6.3 is too vague here. It says "Several methods in the SAGA API support the synchronization of concurrent operations." 2.6.3 is about async ops mostly, and thread concurrency, and I don't think the serve() method should be amongst those affected. It may be better to rename serve(timout) to serve(time-to-wait), to mak clear that is different? Returning a 0 is not a good option in many languages IMHO: the return type is a stream instance - so you would need to return an invalid instance in this case, i.e. one in Error state. That is not easily distinguishable from an instance which got successfully created, but whose connection failed subsequently. What do you think?
- related is the stream::connect method, which now also has a timeout parameter. What should that method do when the timeout expires? Note that it does not have a return value, and that the PostCond: is not fulfilled in that case. So, should it throw a NoSuccessException? That also seems to contradict the above quoted sentence from 2.6.3. Or should the stream state just remain "New"?
The call details say: "on failure, the stream state is changed to âErrorâ" That makes more sense here, IMHO, as the instance already exists, and state needs to be checked in any case.
Also, I have some more questions/remarks now:
Task container
- metric "task_container.state": should be a String, not an Int, since a Saga object id is a string. This was correct in an earlier version, but apparently never made it to the repository.
The metric value is set to the tasks 'cookie' (not object id), which we define to be an int. So, I think it is correct. Makes sense? Having said that, I agree that changing that to object ID would make more sense, as we get rid of the cookie - which now *only* exists for this specific use case (in combination with get_task(int cookie). However, that is an API change, and I am not sure if we should push those, if they are not fixing a real error... Either way, I noted that down for future changes.
- at some point, the add() notes said that it throws an AlreadyExists if the specified task is already a member, but AlreadyExists was not in the throws list. Now the specs say that the addition is silently ignored, which is OK, I guess, but remove() DOES throw a DoesNotExist exception if the task is not a member of the task container.
Yep. The rationale is that adding a task twice will not hurt the application in general: it will still get notified when that task changes, etc. On remove() however, we can assume that the application logic is incorrect. I agree though that this is not symmetric. How much of a problem is it though? Noted it down for later, but do you think this requires fixing right now?
- there is no difference between list_tasks and get_tasks. Should they still both exist?
list_tasks should go away, right.
Buffer
- set_data() and set_size() should also throw NoSuccess since they are equivalent to calling the corresponding version of the constructor, and the constructor throws NoSuccess.
Right. Malloc failure for example should result in NoSuccess I guess...
Namespaces
- ns_entry.remove(): the notes say to things that seem to contradict:
a) if the Recursive flag is defined, the target is recursively removed if it is a directory, otherwise this flag is ignored
b) a BadParameter is thrown if the entry is not a directory and the Recursive flag is set.
*sigh* true. I guess the latter is safer. Your thaughts?
- ns_directory.remove(URL target)
the notes say: if the instance was not closed before, this call performs a close on the instance ....
I don't understand that. Why close the ns_directory when an entry is removed from it? My guess is that this note should be in the ns_entry.remove() notes, not in the ns_directory.remove(URL target) notes.
Correct.
I am trying to finally getting the Java language bindings up-to-par with the current version of the Saga specs. I think I am mostly done with that now, apart from the issues above (for as far as the language bingings are affected).
:-) But isn't it disturbing that issues like the above keep popping up, after so many eyes red the spec, and *implemented* it? *scratch* Many thanks for the thoroughness! :-D Will you be in Brussels for OGF by any chance? Also, on a side note: do you have any thoughts on the message API, which is in public comment right now? Or do you know somebody at VU, or elsewhere, who could give qualified comments? You IBIS guys do IPC all the time, so this is your area of expertiese after all :-D Best, Andre.
Cheers, Ceriel
-- Nothing is ever easy.