Quoting [Ceriel Jacobs] (Feb 25 2009):
Andre Merzky wrote:
Quoting [Ceriel Jacobs] (Feb 25 2009):
I don't know that the c'tor would throw NoSuccess. Maybe a default UserProxy file does not exist, but is set_defaults() the right time to check?
When else can you check? On setting the attribute would be an option - but throwing there would confuse: the exceptions defined on set_attribute are attribute related, not related to the semantics of the context. One could also check internally at the first time the context is being used, but then it is hard to predict on API level when that exception will come, and again it is probably in a context unrelated to the, uhm, context (pun unintented). So, set_defaults provides a good opportnity to flag any inconsistencies.
I would indeed check the first time the context is being used. I don't think it is hard to predict on API level when an exception will come, because virtually all methods can throw a corresponding exception. Also, set_defaults may possibly detect SOME inconsistencies, but certainly not all.
Yes, true, the exceptions listed on set_defaults are almost always listed on other API calls, too (almost though - IncorrectState may be the exception). So, lets try a similar use case to before: 1 saga::context c1 ("ssh"); 2 saga::context c2 ("myproxy"); 3 c2.set_attribute ("Server", "myproxy.teragrid.org/does-not-exist"); 4 5 saga::session s; 6 s.add_context (c1); 7 s.add_context (c2); 8 9 saga::file f (s, "ssh://dead-host.net/data/file"); 10 f.copy ("gridftp://qb.teragrid.org/etc/passwd"); Line 10 would then stumble over the fact that the server I specified for the context does not exist. And an AuthenticationDenied exception would actually fit semantically, too. I agree so far with you. What I don't see is why you have a problem with having set_defaults in the API, as a fixed point to check for that error. Naively, the user could also expect it to get raised on the lines 3, 7, or 9. The calls on line 3 and 7 don't have AuthenticationDenied exception defined. The call on line 9 is not obviously connected to the teragrid. In particular on line 9, the engine may try the myproxy context first, and would raise the misleading AuthenticationDenied exception (*). Also, set_defaults, as motivated earlier, has other uses, too - so it is not like it is going away, really: saga::context c ("globus"); c.set_defaults (); // get info about default globus proxy std::cout << c.get_attribute ("LifeTime"); std::cout << c.get_attribute ("UserID"); std::cout << c.get_attribute ("VO"); without set_defaults, it is totally undefined when these attributes are available. In line 9 above, for example, which is the first 'remote call', the ssh adaptor may succeed, and the myproxy context remains uninitialized, as it's set_defaults() is never called internally. One can very likely achieve a consistent API w/o set_defaults, by defining a couple of semantic changes which address the issues above explicitely, no doubt. I just don't think (a) that this is justified, and (b) that this would really simplify matters. (*) The discussion about exception order chips in here, I know. I assume that the engine would prefer the AuthenticationDenied over the TimeOut of=r NoSuccess exception caused by the dead host. But you can find other examples for other exception ordering policies of course.
Yes, one could argue if an empty type on context creation makes sense, or if one should mandate a valid string. Given that an empty string is a 'valid' string in most languages, that is not much different from other invalid strings like 'klopus'. We have defined error conditions for calling set_defaults() on invalid context attributes. I don't think that this is a big problem at the moment?
I guess there is a difference between "" and "klopus" in that "" is IMHO supposed to mean that the Type is not yet specified, whereas "klopus" specifies a Type, on which the constructor will call set_defaults(), which will probably cause an exception.
Again, set_defaults() in the c'tor cannot work - you could never create a context with a non-default location for the globus proxy file.
Indeed, not if you want set_defaults to do all the things you describe. But I don't think set_defaults is the right place. If you set some attributes explicitly, like "UserProxy", or "UserId", or "UserPass", there is no check either.
That is correct. Setting a single attribute may very well leave the context in an undefined state, like: c.set_attribute ("Type", "myroxy"); c.set_attribute ("UserID", "public"); c.set_attribute ("UserPass", "secret"); c.set_attribute ("Server", "some.server.net"); It would not make sense to complain about an invalid UserID before passwd and server are known. That would be like complaining about invalid executable path when creating a job description, before the target host is known... ;-)
In general, the first time you notice that something is wrong is when you try to actually use the context.
You have a very valid point her: even after set_defaults, there is no guarantee that a context is still valid when a remote operation is actually executed. It may have timed out, for example. However, that is what the AuthenticationFailed exception is for, to flag that the context is unusable. The NoSuccess on set_defaults is rather to signal that the context could not even be created in the first place... Cheers, Andre.
I don't see why this should be any different if you use set_defaults. For instance, you could create an ftp context with perfectly reasonable default values for UserId and UserPass, but if you then connect to an ftp server that does not allow anonymous access, you still have a problem. So, I think a cleaner design would be to not have set_defaults, and to have the context constructor set some reasonable default values (if a Type is specified). It would then be up to the user to make sure that the default values actually work.
-- Nothing is ever easy.