[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