[Patch for Review] iSCSI Name Validation using regex.

Vratislav Podzimek vpodzime at redhat.com
Tue May 5 06:36:18 UTC 2015


>On Wed, 2015-04-29 at 00:19 -0400, Sujith Pandel wrote: 
> Hi Vratislav,
> Thank you for your feedback on this. 
> One of the changes which you mentioned in this:
> 
> > > --- a/pyanaconda/ui/gui/spokes/advstorage/iscsi.py
> > > +++ b/pyanaconda/ui/gui/spokes/advstorage/iscsi.py
> > > @@ -18,7 +18,7 @@
> > >  #
> > >  # Red Hat Author(s): Chris Lumens <clumens at redhat.com>
> > >  #
> > > -
> > > +import re
> > > +from pyanaconda.regexes import ISCSI_IQN_NAME_REGEX, ISCSI_EUI_NAME_REGEX
> > >....
> > > +        #iSCSI Name validation using regex. Name should either match IQN format or EUI format.
> > > +        if re.match(ISCSI_IQN_NAME_REGEX,stripped) or re.match(ISCSI_EUI_NAME_REGEX,stripped):
> > We prefer calling the match() method of the compiled regexes instead of
> > using re.match() on them. That would also remove allow you to get rid of
> > the commas that would otherwise need a space right after them. :)
> > 
> > > +            return True
> > > +        else:
> > > +            return False
> > This if statement should be reduced to just:
> > 
> > return ISCSI_ION_NAME_REGEX.match(stripped) or ISCSI_EUI_NAME_REGEX.match(stripped)
> > 
> > 
> 
> Please note that this results in a run time exception when return value becomes 'None'. Verified this condition.
> (Boolean values are expected at the receiving end.)
> 
> Based on your suggestions above, I am thinking of changing it to:
> 
> if ISCSI_IQN_NAME_REGEX.match(stripped) or ISCSI_EUI_NAME_REGEX.match(stripped):
>     return True
> else:
>     return False
This could/should be done as:
return bool(regex1.match() or regex2.match())

> 
> This will also prevent the need to "import re" in iscsi.py
> Let me know if any changes are required to it.
> 
> > > +import unittest
> > > +import re
> > Seems like the 're' module is not needed anywhere below.
> > 
> Yes. I'll remove it.
> 
> I'll send the modified patch incorporating the changes once you confirm on these changes.
Great, thanks!

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list