* Douglas Landgraf <dougsl...@redhat.com> [2012-09-11 12:15]:
> Hi Ryan,
> 
> On 09/11/2012 10:21 AM, Ryan Harper wrote:
> >* dougsl...@redhat.com<dougsl...@redhat.com>  [2012-09-11 01:38]:
> >>Douglas Schilling Landgraf has uploaded a new change for review.
> >>
> >>Change subject: engine.py: fail if Password doesn't match
> >>......................................................................
> >>
> >>engine.py: fail if Password doesn't match
> >>
> >>Currently, if users in the Node TUI add the password to include the
> >>node through Engine and passwords doesn't match, no failure
> >>message will show and it will continue.
> >>
> >>This patch will show to users a failure message if the passwords
> >>doesn't match.
> >>
> >>Bug-Id: https://bugzilla.redhat.com/show_bug.cgi?id=854457
> >>
> >>Test:
> >>
> >>   * Install Node
> >>   * Select oVirt Engine Tab
> >>   * Add passwords that doesn't match and click 'Apply'
> >>
> >>Change-Id: I143906eb6ce61037418eac25567496c6628aede9
> >>Signed-off-by: Douglas Schilling Landgraf<dougsl...@redhat.com>
> >>---
> >>M vdsm_reg/engine.py.in
> >>1 file changed, 11 insertions(+), 0 deletions(-)
> >>
> >>
> >>   git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/7917/1
> >>
> >>diff --git a/vdsm_reg/engine.py.in b/vdsm_reg/engine.py.in
> >>index 74fe07a..b104635 100644
> >>--- a/vdsm_reg/engine.py.in
> >>+++ b/vdsm_reg/engine.py.in
> >>@@ -42,6 +42,8 @@
> >>  VDSM_REG_CONFIG = "/etc/vdsm-reg/vdsm-reg.conf"
> >>  VDC_HOST_PORT = 443
> >>  TIMEOUT_FIND_HOST_SEC = 5
> >>+PASSWORD_MATCH = 0
> >>+PASSWORD_DOESNT_MATCH = 1
> >>
> >>  fWriteConfig = 0
> >>  def set_defaults():
> >>@@ -186,6 +188,7 @@
> >>          self.root_password_2.setCallback(self.password_check_callback)
> >>          pw_elements.setField(self.root_password_2, 1, 2)
> >>          self.pw_msg = Textbox(60, 6, "", wrap=1)
> >>+        self.pw_resp = PASSWORD_MATCH
> >>
> >>          elements.setField(pw_elements, 0, 6, anchorLeft=1)
> >>          elements.setField(self.pw_msg, 0, 7, padding = (0,0,0,0))
> >>@@ -212,9 +215,17 @@
> >>      def password_check_callback(self):
> >>          resp, msg = password_check(self.root_password_1.value(), 
> >> self.root_password_2.value())
> >>          self.pw_msg.setText(msg)
> >>+        self.pw_resp = resp
> >>          return
> >>
> >>      def action(self):
> >>+        # Show error if the password informed by user doesn't match
> >>+        if self.pw_resp == PASSWORD_DOESNT_MATCH and 
> >>len(self.root_password_1.value())>  0 or \
> >>+            self.pw_resp == PASSWORD_DOESNT_MATCH and 
> >>len(self.root_password_2.value())>  0:
> >>+            msg = "Password Do Not Match!"
> >>+            ButtonChoiceWindow(self.ncs.screen, "@ENGINENAME@", msg, 
> >>buttons = ['Ok'])
> >>+            return
> >>+
> >What about moving the Button popup to an else clause of the if-check
> >that's already validating the password ?
> >
> >         if self.root_password_1.value() != "" and 
> > self.root_password_2.value() != "" and self.root_password_1.value() == 
> > self.root_password_2.value():
> >             # set password ...
> >         else:
> >             ButtonChoiceWindow()
> >             return?
> Adding directly the else statement will break the flow, since it
> requires self.root_password_1.value() and
> self.root_password_2.value() be different of "".
> If user don't want to use such feature, both fields will contain the
> value "" and the flow should continue .


Right, if they're not setting a password at all... I suppose an elif
instead with a check for non-empty password len in either pw1 or 2 would
fix that.


> I am going to send a new patch to avoid a second if statement.

OK

> 
> Looks like you have decided to review the code replying to mailing
> list instead of to use the gerrit flow. Can you please follow the
> gerrit flow as well since currently most of
> developers are using it? At least, until the project change it.

Sure, I'll try to keep current on gerrit as well.

> 
> Thanks for the review.

Sure.

> 
> --
> Cheers
> Douglas

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com

_______________________________________________
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel

Reply via email to