Re: [vdsm] Change in vdsm[master]: engine.py: fail if Password doesn't match

[Ryan Harper]

* dougsland(a)redhat.com <dougsland(a)redhat.com&gt; [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 <dougsland(a)redhat.com&gt;
>  ---
>  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?


>           # To manage the engine_server_port value, use enginePort var
> to avoid
>           # TUI putting the port value in the screen when you are just changing
>           # the value and not asking to draw/print it.
>  
>  
>  --
>  To view, visit http://gerrit.ovirt.org/7917
>  To unsubscribe, visit http://gerrit.ovirt.org/settings
>  
>  Gerrit-MessageType: newchange
>  Gerrit-Change-Id: I143906eb6ce61037418eac25567496c6628aede9
>  Gerrit-PatchSet: 1
>  Gerrit-Project: vdsm
>  Gerrit-Branch: master
>  Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
>  _______________________________________________
>  vdsm-patches mailing list
>  vdsm-patches(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches 
-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh(a)us.ibm.com

Change in vdsm[master]: engine.py: fail if Password doesn't match

[Douglas Schilling Landgraf]

Hi Ryan,

On 09/11/2012 10:21 AM, Ryan Harper wrote:
>  * [e-mail]<dougsland(a)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<dougsland(a)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 .
I am going to send a new patch to avoid a second if statement.

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.

Thanks for the review.

--
Cheers
Douglas

Change in vdsm[master]: engine.py: fail if Password doesn't match

[Ryan Harper]

* Douglas Landgraf <dougsland(a)redhat.com&gt; [2012-09-11 12:15]:
>  Hi Ryan,
>  
>  On 09/11/2012 10:21 AM, Ryan Harper wrote:
>  >* [e-mail]&lt;dougsland(a)redhat.com&gt;  [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&lt;dougsland(a)redhat.com&gt;
>  >>---
>  >>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
ryanh(a)us.ibm.com