On 4 April 2013 10:03, Helga Velroyen <hel...@google.com> wrote: > Hi! > > > On Wed, Apr 3, 2013 at 6:19 PM, Bernardo Dal Seno <bdals...@google.com> > wrote: >> >> On 28 March 2013 15:50, Helga Velroyen <hel...@google.com> wrote: >> > This patch is a preparation for later patches in QA in this series >> > it refactors the instance QA to make it more flexible regarding >> > creation of instances with different disk templates. Right now we >> > only support creation of instances with disk templates 'drbd', 'plain', >> > and 'diskless'. With our current plans to improve storage handling, >> > we should make QA more flexible to also create instances of other >> > templates. This patch restructures the code in a way that instances >> > can be created more easily to be used in other QA scripts. >> > >> > Signed-off-by: Helga Velroyen <hel...@google.com> >> > --- >> > qa/qa_instance.py | 88 >> > ++++++++++++++++++++++++++++++++++++++++++++++++++----- >> > 1 file changed, 80 insertions(+), 8 deletions(-) >> > >> > diff --git a/qa/qa_instance.py b/qa/qa_instance.py >> > index f1c2ab7..9ca36eb 100644 >> > --- a/qa/qa_instance.py >> > +++ b/qa/qa_instance.py >> > @@ -93,6 +105,68 @@ def _DiskTest(node, disk_template, fail=False): >> > return None >> > >> > >> > +def _CreateInstanceByDiskTemplateOneNode(nodes, disk_template, >> > fail=False): >> > + """Creates an instance using the given disk template for disk >> > templates >> > + for which one given node is sufficient. These templates are for >> > example: >> > + plain, diskless, file, sharedfile, blockdev, rados. >> > + >> > + @type nodes: list of nodes >> > + @param nodes: a list of nodes, whose first element is used to create >> > the >> > + instance >> > + @type disk_template: string >> > + @param disk_template: the disk template to be used by the instance >> > + @return: the created instance >> > + >> > + """ >> > + assert len(nodes) > 0 >> >> Why not "assert len(nodes) == 1"? > > > Because to keep it more flexible. It should be possible to give it more than > one node and still work properly. The function just uses the first one and > ignores the rest. This was more elegant when testing all disk templates and > calling the "CreateInstanceByDiskTemplate" function which then selects the > correct subfunction (of which is one this
I see your point. Still I don't like when a function silently ignores its input, as too often that hides an error, but since you've already used this feature, I'd say it's okay to leave it this way. Thanks, Bernardo
