On 05/11/10 10:40 AM, Dermot McCluskey wrote:
Jean,
Comments inline below.
On 05/11/10 16:57, jean.mccormack wrote:
On 05/11/10 07:44 AM, Dermot McCluskey wrote:
Jean,
I think this is a very useful exercise:
What I don't see in this proposal in the interaction with the cache.
I'd like to see that, even in pseudo-code form, so I'm sure we're on
the same page.
Yes. Let's take it that step further.
I don't understand why you pass 2 params into register_checkpoint,
which
seem to consist of a function pointer and an object which is a
sub-class
of DataObject. If you've added this object to the cache, you should
not
need to pass it around as a param - it should only be accessed via the
cache.
Yes. The more I thought about this last night the less I liked that too.
I'm thinking we don't need it. The data is in the cache and we can
access that
so we should do so. register_checkpoint should be as it was:
register_checkpoint(name, func)
Further comments in-line below.
On 05/10/10 18:21, jean.mccormack wrote:
During the last prototype meeting, Dave and I were tasked with
figuring out the DOC interface to pass data to the checkpoint
modules. We met to discuss this last week with Evan and Sanjay
attending for portions of the meeting to help.
Input on this proposal is requested from Dermot, Darren, Karen and
Sarah but anyone else is welcome to respond.
There will be a class CheckpointNode that will be an ABC and will
inherit from DataObject. It will have a name attribute.
Each type of checkpoint (TI, TD, Transfer etc) will have it's own
subclass of CheckpointNode that will have attributes
specific to that type of checkpoint.
The xml that will be generated would look like this:
<checkpoint>
<name>"name of checkpoint"</name>
<checkpoint specific attributes to be defined by each checkpoint>
</checkpoint>
Is this just for illustration? In reality, these checkpoints would
not be shown in the manifest, and therefore would not generate any
XML, right?
Not sure. They are in the DOC, whether they get written to the manifest
is not really part of this proposal. Now it could be useful debug
information
but I would leave that decision up to whomever (you?) is doing that
work.
To further explain this I'll use an example of a client that does
TD, TI, TI, Transfer, Transfer. Note this is not meant
to be any real code, it's more pseudocode than anything.
Client()
td_node = TD_ChkptNode("TargetDiscovery")
ti_node1 = TI_ChkptNode("TI_IPS")
ti_node2 = TI_ChkptNode("TI_CPIO")
xfer_node1 = Xfer_ChkptNode("XFER_IPS")
xfer_node2 = Xfer_ChkptNode("XFER_CPIO")
...
TD = register_checkpoint(td.discover, td_node)
TI1 = register_checkpoint(ti.instantiate, ti_node1)
TI2 = register_checkpoint(ti.instantiate, ti_node2)
Xfer1 = register_checkpoint(xfer.transfer, xfer_node1)
Xfer2 = register_checkpoint(xfer.transfer, xfer_node2)
What type of entity is the return value from register_checkpoint
(TD, TI1, etc)? From the prototype, I recall that the engine
does not instantiate the actual checkpoint objects until it's
ready to execute them, so the checkpoint objects don't exist
at this stage.
I believe register checkpoint does instantiate the object.
Not in the prototype. register_checkpoint() instantiates the
CheckpointData objects, but not the Checkpoints themselves. They
are not created until CheckpointData.load_checkpoint() is called,
which is shortly before execute() is called.
Keith explained before that he wants to keep it this way, as it's
more efficient, especially in the case where you don't actually
get to run all the registered checkpoints.
Also, register_checkpoint doesn't return anything in the prototype.
OK. Thanks. Our mistake here. So we would register and then the
TD, TI1.... would be returned from the load_checkpoint() call.
But the call to register_checkpoint should not be as indicated above,
but rather
as the prototype did, name and function.
# Because we bounce out of engine after TD runs we may want to
tell TD where to put things.
td_node.dst = "Discovered Targets" # TD_ChkptNode has property
"dst"
I would have expected that the name of the root node where TD stores
targets is a global constant, rather than a value that needs to be
stored and passed around.
It should be programmable for the reason that if you have 2 TD
modules being
executed then you probably wouldn't want the 2nd tromping on the
first's output.
This would also make debug easier in the case of multiple TD's, you'd
have the
output from both checkpoints available.
However, this is an interface that Dave needs to design yet so it's a
bit squishy.
When would you ever need to run several different TDs?
Not sure exactly of the use case. We could make the dst a predefined
value and if the
situation arises change it. I'm fine with that. My idea was to try to
make everything as
generic as possible.
which is just a name.
# this name is the name it
will give the root node of
# a tree of nodes, Physical
and Logical that it discoveres.
td_node.start = "..." # maybe for DC we don't want it to do
physical target discovery...
I don't understand the above line?
Just setting a fake td specific attribute.
# Now execute engine just running TD:
execute(TD)
# And now we need to add information to other nodes
ti_node1.create = ... # root node of some tree of nodes that
the App wants
ti_node2.create = ... # root node of some tree of nodes that
the App wants
xfer_node1.src ="http://some/ips/repo:port"
xfer_node1.dst = "rpool/jean/pkg_imag" # image area for IPS to
install to.
xfer_node2.src = "/"
xfer_node2.dst = "rpool/jean/whatever" # area to cpio to
So here we would need to make sure the data is actually getting into
the cache.
This is missing from this example. Maybe something like:
ti_node1.add_data_to_cache()
ti_node2.add_data_to_cache()
xfer_node1.add_data_to_cache()
xfer_node2.add_data_to_cache()
I think it would be more like:
doc = DataObjectCache.get_instance()
doc.add_child(ti_node1)
doc.add_child(ti_node2)
doc.add_child(xfer_node1)
doc.add_child(xfer_node2)
OK. I guess the real point is it has to get there somehow, right? The
proposal was missing that piece.
Of course it was in my head but that didn't really do you all much good
now did it?
# And execute remaining checkpoints:
execute(TI1, TI2, Xfer1, Xfer2) #<---- execute the rest of the
checkpoints.
I'd like to see an example that shows data being retrieved from the
cache
within a checkpoint.
OK. Let's take Xfer. The execute for the transfer module would get
the cache via
doc = DataObject.get_instance()
node = doc.get_child_by_name(self.name)
Assuming self.name matches the strings "XFER_IPS" or
"XFER_CPIO" from earlier, then Yes.
Yes it would have to.
And then pull the appropriate transfer specific information from this
node.
Of course you were probably confused here because the write to the
cache was
missing from the example.
Does this help?
Yes - but I still don't understand some of this.
Originally, I was assuming that your CheckpointNode was a replacement
for the CheckpointData objects that the Engine uses in the current
prototype. Now I see they are separate from, and in addition to,
CheckpointData, right?
Yes.
So, essentially, the Application wants to create some parameter-type
data for the checkpoints at the time of registering, and the checkpoints
wish to retrieve this data when they are being executed, right?
Yes.
How many variables would there ever be? If it's mostly just CPIO vs
IPS values, then I prefer Darren's suggestion of having separate
checkpoint *classes* for each type. But if there's enough potential
checkpoint params required, then the approach above will work.
(I'd still like to see an updated proposal to confirm.)
Number of variables is unknown but just from looking at transfer, if the
client decided
to do multiple cpio's in one checkpoint instantiation, that could get on
the largish side.
There would be separate classes for each type. They would subclass from
CheckpointNode
with additional attributes.
So what I would see is something on the order of this:
class CheckpointNode(name):
_name
class IPSNode(CheckpointNode):
def __init__(self, name, type, param1, param2,
.....): # not sure if specific args should be here or below.
CheckpointNode.__init__(self, name)
_type = type
_param1 = param1
_param2 = param2
def add_data_to_cache(): # maybe allow this to take the
specific args rather than init
doc = DataObject.get_instance()
node = doc.get_child_by_name(self._name)
rest of code to add the specifc params to the doc
Is this what you mean? Or are you saying that you're not sure if IPSNode
needs to subclass CheckpointNode?
Since a checkpoint node MUST have a name that led us to the subclassing.
We will definitely update the proposal after the feedback comes in.
Jean
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss