Hi Karen, More responses, with some parts cut out where I've no more comment...
On 06/10/10 12:17 AM, Karen Tung wrote: > On 06/09/10 02:24, Darren Kenny wrote: >> Section 4, page 5: >> I'm wondering if the programming language to use is really a choice for >> this project since it's been decided at the higher level of the CUD >> design >> that Python is the language of choice for new implementations. >> > I didn't see any specific mention of Python in the architecture design doc. > It only talks about using an Object Oriented language. So, I thought I will > be complete, and talk about other potential choices here. That's true, but I feel that this is something that should be fixed in the architecture document rather than here since it's certainly the assumption that the majority of people are working on. > > I think I will give more description on each of the required > to implement methods, and make a reference to section 10 > about what type of value to return. From the comments I received, > looks like everybody want more information about what kind of > value should be returned for progress, and how that value is > determined., I will try to have more details in section 10. Perfect... > Section 6.4.2 talks about the fact that the execution is > done in a separate thread. In the 2nd paragraph, it explicitly > talks about the fact that the execute() function will not return > until after all checkpoints are executed successfully or one of > the checkpoint failed. So, we don't need a "completed" call-back. Hmm, somehow I missed that piece the mention of threading there, sorry. > Relying on progress monitoring might not be a good idea, because > if the application didn't register progress monitor with the logger, > or if the checkpoints don't report their progress to the logger, the > application > won't know the progress. Agreed. > > If the application want to do something else when execute_checkpoint() is > running, it should run execute_checkpoint() in a thread. Ok, but I wonder if this is an extra level of threading that really shouldn't be needed? For example, right now, the GUI doesn't need to run liborchestrator in a separate thread when doing TD, it's something that's hidden from the GUI, a change like this means that the GUI needs to add another thread-level. (It's more of an example really, since the GUI is probably going to need re-work anyway to work with the new CUD). It just seems that the Engine would seem to be the logical place to manage the threading of things, since it's likely to be doing threading anyway... > >> I think a section on threading would be useful, since I believe it has >> an >> impact on other elements of the install infrastructure. >> > Good idea. Let me add a section that talks about this. Cool, certainly it's something worth making VERY clear :) >> Section 6.1, page 7: >> The DOC is not a singleton any more, and there will be a need of a >> "get_data_object_cache()" method on the Engine. >> > Do you think we need to define a function that just returns the variable? > We can allow people to access the doc variable directly, and make the > name of the variable an interface. We can use the "property" feature > in Python if we found that we need to define a function later. Sure, probably engine.data_object_cache property would be fine, and I think it should be read-only too... > >> Section 6.3.1, page 8: >> Maybe the use of the term "dangerous" is too severe? It might be better >> to >> be specific about the risk, and how it might be able to be prevented. >> > I can talk about risk, but is there really a way to prevent it? > We do allow checkpoints to exist everywhere. Do we want to restrict it > to loading from a certain location? I think it's more how people implementing checkpoints and using the Engine could mitigate the risk, if at all possible - e.g. we could make the use of a directory path a high-risk while using a module name - to be loaded from sys.path - would be considered safer? Maybe thus producing a warning if it's seen to be a full path, or something... >> Section 6.7.1, page 12: >> How are you going to guarantee that removal of snapshots from /tmp will >> not remove snapshots from other processes - e.g. are you using a dir for >> each run (/tmp/install_snapshots.PID/), just might be worth spelling >> out. >> > Section 7.4, the second bullet point did talk about using PID for DOC > snapshots that are stored in /tmp. Do you think that's sufficient? I think that's fine, but I would prefer to see things put in a sub-directory - especially in the case of DC - to avoid lots of messy files in /tmp. (Dave has mentioned the possible mis-use of /tmp). >> Section 7.5, page 15& 16: >> How is "DataObjectCacheNotEmpty" state determined? Should the DOC >> provide >> a is_empty() method is or just checking that len(doc.children) == 0, >> enough? >> > I was going to use doc.get_descendants() to make sure it return > an empty list. Looks like I can use len(doc.children) too. > Are they both the same? Is one better than the other? The call to get_descendants() would be (potentially) an expensive operation if the tree isn't empty - I will provide a suitable method for you, especially given that there are two sub-trees, e.g.: doc.persistent.has_children (which is really the one you want to check for roll-backs...) Thanks, Darren. _______________________________________________ caiman-discuss mailing list caiman-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/caiman-discuss