[PATCH] Add support for NFS as install source in TUI. (#971298)

Samantha N. Bueno sbueno+anaconda at redhat.com
Tue Jul 23 15:22:05 UTC 2013


On Mon, Jul 22, 2013 at 05:40:12PM -0700, Brian C. Lane wrote:
> On Mon, Jul 22, 2013 at 11:28:42AM -0400, Samantha N. Bueno wrote:
> > Note: should go into rhel7-branch also.
> > 
> > Resolves: rhbz#971298
> > ---
> >  pyanaconda/ui/tui/spokes/source.py | 68 ++++++++++++++++++++++++++++++++++----
> >  1 file changed, 62 insertions(+), 6 deletions(-)
> > 
> > diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py
> > index cd5f718..3714364 100644
> > --- a/pyanaconda/ui/tui/spokes/source.py
> > +++ b/pyanaconda/ui/tui/spokes/source.py
> > @@ -32,6 +32,7 @@ from pyanaconda.constants import THREAD_SOURCE_WATCHER, THREAD_SOFTWARE_WATCHER,
> >  from pyanaconda.constants import THREAD_PAYLOAD_MD, THREAD_STORAGE, THREAD_CHECK_SOFTWARE
> >  
> >  import re
> > +from .... import time
> 
> why not 'import time'?

Check my reply to vpodzime, I responded to this there.
 
> >      def getRepoMetadata(self):
> >          """ Pull down yum repo metadata """
> > +        # unfortunately, this sleep is needed in order to prevent a tb
> > +        # anaconda needs to try and mount our nfs dir (if we're using nfs)
> > +        # before kicking off all of the yum crap
> > +        time.sleep(1)
> 
> ugh. Is 1 second always enough? Oh, I see we do this in GUI so that the
> hubQ message has time to make it through, so maybe this isn't needed in
> TUI?

I also responded to this in my reply to vpodzime since he pointed it out
as well, but you are right--the sleep is not required. Thanks for
catching that.
 
> > +
> > +class SpecifyNFSRepoSpoke(EditTUISpoke):
> > +    """ Specify the repo URL here if closest mirror not selected. """
> > +    title = _("Specify Repo Options")
> > +    category = "source"
> > +
> > +    edit_fields = [
> > +        Entry(_("Repo url"), "url", re.compile(".*$"), True),
> 
> Instead of calling it a url, maybe say: "NFS <server>:/<path>"

Good idea; fixed locally.
 
> > +    def apply(self):
> > +        """ Apply our changes. """
> > +        if self.args.url == "" or not ':' in self.args.url:
> > +            return False
> > +
> > +        if self.args.url.startswith("nfs://"):
> > +            self.args.url = self.args.url.strip("nfs://")
> > +
> > +        try:
> > +            (self.data.method.server, self.data.method.dir) = self.args.url.split(":", 2)
> > +        except ValueError as err:
> > +            LOG.error("ValueError: %s" % (err,))
> > +            self.errors.append("Failed to set up installation source")
> > +            return
> > +
> > +        self.data.method.opts = self.args.opts or ""
> 
> It may make sense to use the nfs format that we use everywhere else, and
> let iutil.parseNfsUrl handle splitting things instead of letting them
> enter the server/path and options in 2 places.

Hmm, I'm not too sure I understand what you mean WRT entering the
server/path and options in two places. The logic is fairly similar to
how this is handled in the GUI--the server/path and options are both
entered separately there as well, as is the splitting of server and
path into their respective variables.

Thanks for looking this over!

Samantha


More information about the anaconda-patches mailing list