| Message ID | add0772aff30943193addd6de8c698640927fe1d.1243179847.git.mgoldish@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote: > kvm_vm.py: add function VM.get_name(). > kvm_preprocessing.py: use VM.get_name() instead of directly accessing the .name > attribute. Are there any advantages of creating this method over directly accessing the attribute? > Signed-off-by: Michael Goldish <mgoldish@redhat.com> > --- > client/tests/kvm_runtest_2/kvm_preprocessing.py | 6 +++--- > client/tests/kvm_runtest_2/kvm_vm.py | 4 ++++ > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py b/client/tests/kvm_runtest_2/kvm_preprocessing.py > index c9eb35d..bcabf5a 100644 > --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py > +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py > @@ -178,7 +178,7 @@ def preprocess(test, params, env): > if vm.is_dead(): > continue > if not vm.verify_process_identity(): > - kvm_log.debug("VM '%s' seems to have been replaced by another process" % vm.name) > + kvm_log.debug("VM '%s' seems to have been replaced by another process" % vm.get_name()) > vm.pid = None > > # Destroy and remove VMs that are no longer needed in the environment > @@ -187,8 +187,8 @@ def preprocess(test, params, env): > vm = env[key] > if not kvm_utils.is_vm(vm): > continue > - if not vm.name in requested_vms: > - kvm_log.debug("VM '%s' found in environment but not required for test; removing it..." % vm.name) > + if not vm.get_name() in requested_vms: > + kvm_log.debug("VM '%s' found in environment but not required for test; removing it..." % vm.get_name()) > vm.destroy() > del env[key] > > diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py > index fab839f..df99859 100644 > --- a/client/tests/kvm_runtest_2/kvm_vm.py > +++ b/client/tests/kvm_runtest_2/kvm_vm.py > @@ -454,6 +454,10 @@ class VM: > """Return True iff the VM's PID does not exist.""" > return not kvm_utils.pid_exists(self.pid) > > + def get_name(self): > + """Return the VM's name.""" > + return self.name > + > def get_params(self): > """Return the VM's params dict. >
----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote: > On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote: > > kvm_vm.py: add function VM.get_name(). > > kvm_preprocessing.py: use VM.get_name() instead of directly > accessing the .name > > attribute. > > Are there any advantages of creating this method over directly > accessing > the attribute? Not really, it's just that everywhere else we use "get" interface functions and I like to be consistent. I also thought it was good OOP practice to make the interface independent of the internal implementation. For example, we may choose not to store the name attribute in the class instance, but rather retrieve it or generate it (if it gets more complex) from the params upon request. > > Signed-off-by: Michael Goldish <mgoldish@redhat.com> > > --- > > client/tests/kvm_runtest_2/kvm_preprocessing.py | 6 +++--- > > client/tests/kvm_runtest_2/kvm_vm.py | 4 ++++ > > 2 files changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py > b/client/tests/kvm_runtest_2/kvm_preprocessing.py > > index c9eb35d..bcabf5a 100644 > > --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py > > +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py > > @@ -178,7 +178,7 @@ def preprocess(test, params, env): > > if vm.is_dead(): > > continue > > if not vm.verify_process_identity(): > > - kvm_log.debug("VM '%s' seems to have been replaced by > another process" % vm.name) > > + kvm_log.debug("VM '%s' seems to have been replaced by > another process" % vm.get_name()) > > vm.pid = None > > > > # Destroy and remove VMs that are no longer needed in the > environment > > @@ -187,8 +187,8 @@ def preprocess(test, params, env): > > vm = env[key] > > if not kvm_utils.is_vm(vm): > > continue > > - if not vm.name in requested_vms: > > - kvm_log.debug("VM '%s' found in environment but not > required for test; removing it..." % vm.name) > > + if not vm.get_name() in requested_vms: > > + kvm_log.debug("VM '%s' found in environment but not > required for test; removing it..." % vm.get_name()) > > vm.destroy() > > del env[key] > > > > diff --git a/client/tests/kvm_runtest_2/kvm_vm.py > b/client/tests/kvm_runtest_2/kvm_vm.py > > index fab839f..df99859 100644 > > --- a/client/tests/kvm_runtest_2/kvm_vm.py > > +++ b/client/tests/kvm_runtest_2/kvm_vm.py > > @@ -454,6 +454,10 @@ class VM: > > """Return True iff the VM's PID does not exist.""" > > return not kvm_utils.pid_exists(self.pid) > > > > + def get_name(self): > > + """Return the VM's name.""" > > + return self.name > > + > > def get_params(self): > > """Return the VM's params dict. > > > -- > Lucas Meneghel Rodrigues > Software Engineer (QE) > Red Hat - Emerging Technologies -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2009-06-03 at 01:01 -0400, Michael Goldish wrote: > ----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote: > > > On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote: > > > kvm_vm.py: add function VM.get_name(). > > > kvm_preprocessing.py: use VM.get_name() instead of directly > > accessing the .name > > > attribute. > > > > Are there any advantages of creating this method over directly > > accessing > > the attribute? > > Not really, it's just that everywhere else we use "get" interface > functions and I like to be consistent. I also thought it was good > OOP practice to make the interface independent of the internal > implementation. For example, we may choose not to store the name > attribute in the class instance, but rather retrieve it or generate > it (if it gets more complex) from the params upon request. Fair enough, now that you've put it that way I agree with you. > > > Signed-off-by: Michael Goldish <mgoldish@redhat.com> > > > --- > > > client/tests/kvm_runtest_2/kvm_preprocessing.py | 6 +++--- > > > client/tests/kvm_runtest_2/kvm_vm.py | 4 ++++ > > > 2 files changed, 7 insertions(+), 3 deletions(-) > > > > > > diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py > > b/client/tests/kvm_runtest_2/kvm_preprocessing.py > > > index c9eb35d..bcabf5a 100644 > > > --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py > > > +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py > > > @@ -178,7 +178,7 @@ def preprocess(test, params, env): > > > if vm.is_dead(): > > > continue > > > if not vm.verify_process_identity(): > > > - kvm_log.debug("VM '%s' seems to have been replaced by > > another process" % vm.name) > > > + kvm_log.debug("VM '%s' seems to have been replaced by > > another process" % vm.get_name()) > > > vm.pid = None > > > > > > # Destroy and remove VMs that are no longer needed in the > > environment > > > @@ -187,8 +187,8 @@ def preprocess(test, params, env): > > > vm = env[key] > > > if not kvm_utils.is_vm(vm): > > > continue > > > - if not vm.name in requested_vms: > > > - kvm_log.debug("VM '%s' found in environment but not > > required for test; removing it..." % vm.name) > > > + if not vm.get_name() in requested_vms: > > > + kvm_log.debug("VM '%s' found in environment but not > > required for test; removing it..." % vm.get_name()) > > > vm.destroy() > > > del env[key] > > > > > > diff --git a/client/tests/kvm_runtest_2/kvm_vm.py > > b/client/tests/kvm_runtest_2/kvm_vm.py > > > index fab839f..df99859 100644 > > > --- a/client/tests/kvm_runtest_2/kvm_vm.py > > > +++ b/client/tests/kvm_runtest_2/kvm_vm.py > > > @@ -454,6 +454,10 @@ class VM: > > > """Return True iff the VM's PID does not exist.""" > > > return not kvm_utils.pid_exists(self.pid) > > > > > > + def get_name(self): > > > + """Return the VM's name.""" > > > + return self.name > > > + > > > def get_params(self): > > > """Return the VM's params dict. > > > > > -- > > Lucas Meneghel Rodrigues > > Software Engineer (QE) > > Red Hat - Emerging Technologies
diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py b/client/tests/kvm_runtest_2/kvm_preprocessing.py index c9eb35d..bcabf5a 100644 --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py @@ -178,7 +178,7 @@ def preprocess(test, params, env): if vm.is_dead(): continue if not vm.verify_process_identity(): - kvm_log.debug("VM '%s' seems to have been replaced by another process" % vm.name) + kvm_log.debug("VM '%s' seems to have been replaced by another process" % vm.get_name()) vm.pid = None # Destroy and remove VMs that are no longer needed in the environment @@ -187,8 +187,8 @@ def preprocess(test, params, env): vm = env[key] if not kvm_utils.is_vm(vm): continue - if not vm.name in requested_vms: - kvm_log.debug("VM '%s' found in environment but not required for test; removing it..." % vm.name) + if not vm.get_name() in requested_vms: + kvm_log.debug("VM '%s' found in environment but not required for test; removing it..." % vm.get_name()) vm.destroy() del env[key] diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py index fab839f..df99859 100644 --- a/client/tests/kvm_runtest_2/kvm_vm.py +++ b/client/tests/kvm_runtest_2/kvm_vm.py @@ -454,6 +454,10 @@ class VM: """Return True iff the VM's PID does not exist.""" return not kvm_utils.pid_exists(self.pid) + def get_name(self): + """Return the VM's name.""" + return self.name + def get_params(self): """Return the VM's params dict.
kvm_vm.py: add function VM.get_name(). kvm_preprocessing.py: use VM.get_name() instead of directly accessing the .name attribute. Signed-off-by: Michael Goldish <mgoldish@redhat.com> --- client/tests/kvm_runtest_2/kvm_preprocessing.py | 6 +++--- client/tests/kvm_runtest_2/kvm_vm.py | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-)