Merge lp:~nuclearbob/utah/url-config into lp:utah

Proposed by Max Brustkern
Status: Merged
Merged at revision: 776
Proposed branch: lp:~nuclearbob/utah/url-config
Merge into: lp:utah
Diff against target: 44 lines (+13/-4)
1 file modified
utah/config.py (+13/-4)
To merge this branch: bzr merge lp:~nuclearbob/utah/url-config
Reviewer Review Type Date Requested Status
Javier Collado (community) Approve
Max Brustkern (community) Needs Resubmitting
Review via email: mp+138825@code.launchpad.net

Description of the change

This branch uses url_argument to allow the use of remote config files. It will allow a script in a test branch to specify a config file via an lp: address, which should be useful in fixing lp:1087679.

To post a comment you must log in.
Revision history for this message
Max Brustkern (nuclearbob) wrote :

This merge creates an issue when a config directory is specified that does not exist, i.e., /etc/utah/conf.d. I'm fixing that now.

review: Needs Fixing
lp:~nuclearbob/utah/url-config updated
775. By Max Brustkern

Changed routine to work with non-existent directories

Revision history for this message
Max Brustkern (nuclearbob) wrote :

I've made some changes to address that issue and retested. I'm retesting the bugfix now.

review: Needs Resubmitting
Revision history for this message
Javier Collado (javier.collado) wrote :

Due to the way `url_argument` works, for URLs other than `lp` ones, you can get
the same URL back

>>> url_argument('http://www.ubuntu.com')
'http://www.ubuntu.com'

If you just want to add support for `lp` URLs, then I'd advice
to add simple check like:
elif abspath.startswith('lp:')

to avoid problems with other URLs.

If you want to add support for `http` URLs, then a wrapper can be added to the
`utah.url` module to use `urlretrieve` when the value returned from
`urlargument` isn't a path to a local file.

Revision history for this message
Javier Collado (javier.collado) wrote :

Since this is related to a critical bug, I'm merging it using the first
alternative in my previous comment (abspath.startswith('lp:')).

If further work is needed to support other URLs, it can be addressed in a
separate bug.

review: Approve
Revision history for this message
Max Brustkern (nuclearbob) wrote :

Thanks for the fix.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'utah/config.py'
--- utah/config.py 2012-12-03 15:49:35 +0000
+++ utah/config.py 2012-12-07 20:41:20 +0000
@@ -29,6 +29,10 @@
29import subprocess29import subprocess
30import sys30import sys
3131
32from argparse import ArgumentTypeError
33
34from utah.url import url_argument
35
3236
33def getbridge():37def getbridge():
34 """38 """
@@ -171,7 +175,7 @@
171 else:175 else:
172 # Otherwise, default to i386176 # Otherwise, default to i386
173 LOCALDEFAULTS['arch'] = 'i386'177 LOCALDEFAULTS['arch'] = 'i386'
174 178
175# I'm not sure lsb_release is always around179# I'm not sure lsb_release is always around
176with open(os.devnull, 'w') as fnull:180with open(os.devnull, 'w') as fnull:
177 if subprocess.call(['which', 'lsb_release'],181 if subprocess.call(['which', 'lsb_release'],
@@ -203,12 +207,17 @@
203for path in paths:207for path in paths:
204 if path is not None:208 if path is not None:
205 abspath = os.path.expanduser(path)209 abspath = os.path.expanduser(path)
206 if os.path.isfile(abspath):210 if os.path.isdir(abspath):
207 files.append(abspath)
208 elif os.path.isdir(abspath):
209 for walkdir, _dirs, walkfiles in os.walk(abspath):211 for walkdir, _dirs, walkfiles in os.walk(abspath):
210 for walkfile in walkfiles:212 for walkfile in walkfiles:
211 files.append(os.path.join(abspath, walkdir, walkfile))213 files.append(os.path.join(abspath, walkdir, walkfile))
214 elif os.path.isfile(abspath):
215 files.append(abspath)
216 else:
217 try:
218 files.append(url_argument(abspath))
219 except ArgumentTypeError:
220 pass
212for conffile in files:221for conffile in files:
213 if os.path.isfile(conffile) and os.path.getsize(conffile) > 0:222 if os.path.isfile(conffile) and os.path.getsize(conffile) > 0:
214 with open(conffile) as fp:223 with open(conffile) as fp:

Subscribers

People subscribed via source and target branches