On 05/11/10 11:46 AM, Dermot McCluskey wrote:
Jean,
See below.
On 05/11/10 18:08, jean.mccormack wrote:
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.
No - I was referring to Darren's suggestion of having different
Checkpoint sub-classes for TI_CPIO and TI_IPS, etc, and doing
away with the need for CheckpointNode entirely. But I see
that you are not adopting this suggestion.
So what I would see is something on the order of this:
class CheckpointNode(name):
shouldn't this sub-class DataObject?
Yup. Sorry.
_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
Adding data to the cache is an operation on the cache, not on the
data. Also, it's the entire CheckpointNode object that you
want to put in the cache - not just the specific attributes
within that object. So, the pseduo-code is something like:
Application:
trans_data = CheckpointNode("XFER_IPS", type, param1, ...)
# or, optionally:
# trans_data = CheckpointNode("XFER_IPS")
# trans_data.type = type
# trans_data.param1 = param1 ...
doc = DataObjectCache.get_instance()
doc.add_child(trans_data)
# that's it - it's all in the cache now
Later, in TD:
doc = DataObjectCache.get_instance()
# there are several ways to get data from cache, this is just one:
trans_data = doc.get_first_child_by_name_and_type("XFER_IPS",
CheckpointNode)
if trans_data is None:
raise Exception("CheckpointNode for XFER_IPS is missing")
my_type = trans_data.type
my_param1 = trans_data.param1
With some qualifications that it might not be this flat of a structure which
is why I was allowing a method to do it. But we can always work that out.
Jean
...
- Dermot
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
