Merge lp:~nuclearbob/utah/bug1147306 into lp:utah

Proposed by Max Brustkern
Status: Merged
Merged at revision: 828
Proposed branch: lp:~nuclearbob/utah/bug1147306
Merge into: lp:utah
Diff against target: 44 lines (+11/-2)
2 files modified
debian/changelog (+6/-2)
utah/provisioning/ssh.py (+5/-0)
To merge this branch: bzr merge lp:~nuclearbob/utah/bug1147306
Reviewer Review Type Date Requested Status
Javier Collado (community) Approve
Max Brustkern (community) Needs Resubmitting
Review via email: mp+151806@code.launchpad.net

Description of the change

It looks like the templating change broke the ProvisionedMachine class. This change fixed it for me. We should figure out a good way to add ProvisionedMachine to the regular testing as well.

To post a comment you must log in.
Revision history for this message
Andy Doan (doanac) wrote :

Its ugly, but I'm +1 for now. In the future we can hopefully make this more sane by getting the installer logic not bound so tightly to machine.py.

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

Note that for provisioned machines only "install-deb-command.jinja2" is used to
install the client package and that the "add_backslash" filter is only used by
"utah-latecommand-in-target.jinja2". Hence, lines 20-25 in the diff can be removed
since they aren't needed.

We could also have a new module that takes care of the template environment to
avoid the duplication of the filter code, but for now I think that probably
that's an overkill unless we find a case in which the filter code duplication
is really needed.

By the way, for all the other machines there shouldn't be any problem since
ProvisionedMachine is the only machine class that doesn't call the
Machine.__init__ method through chained super calls. In the future, it would be
nice to have a class hierarchy in which the initialization method is reused for
all subclasses.

I need to add provisioned machines to the set of basic tests that I run for
each change to make sure things aren't broken like in this case.

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

One more thing. When the 0.8 version was released, I started the 0.9 version in
changelog in the dev branch and marked it as unreleased. The plan is to to mark
it as released when the changes are merged to trunk.

Anyway, the point I'm trying to make is that I've been adding a description of
all big changes, in particular bug fixes, that have been merged into dev, so
that it's easier to know what's not included yet in the version from trunk.
Hence, please add such a description to changelog. Thanks.

lp:~nuclearbob/utah/bug1147306 updated
828. By Max Brustkern

Removed unneeded extra template method

829. By Max Brustkern

Fixed ProvisionedMachine template support

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

I think both of those should be addressed now.

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

Sorry for being picky, but for consistency in other changelog (and bzr log)
entries, I think you should use:
<email address hidden>"
DEBFULLNAME="Max Brustkern"

Also please add "LP: #1147306" to the changelog entry.

lp:~nuclearbob/utah/bug1147306 updated
830. By Max Brustkern

Updated changelog

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

I'm not sure why those variables didn't get set, I'll have to check my .bashrc. I updated that manually and added the bug number. Thanks for noticing that.

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

Changes working for me. Thanks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2013-03-04 14:46:39 +0000
+++ debian/changelog 2013-03-08 13:53:21 +0000
@@ -1,12 +1,16 @@
1utah (0.9ubuntu1) UNRELEASED; urgency=low1utah (0.9ubuntu2) UNRELEASED; urgency=low
22
3 [ Javier Collado ]
3 * Added battery_measurements boolean variable to runlist (defaults to False)4 * Added battery_measurements boolean variable to runlist (defaults to False)
4 * Added success command failure detection (LP: #1126115)5 * Added success command failure detection (LP: #1126115)
5 * Added installation failure log message (LP: #1126361)6 * Added installation failure log message (LP: #1126361)
6 * Added check to make sure user is in the password database7 * Added check to make sure user is in the password database
7 (LP: #1071020)8 (LP: #1071020)
89
9 -- Javier Collado <javier.collado@canonical.com> Wed, 27 Feb 2013 09:28:58 +010010 [ Max Brustkern ]
11 * Fixed ProvisionedMachine template support (LP: #1147306)
12
13 -- max <max@canonical.com> Thu, 07 Mar 2013 17:11:12 -0500
1014
11utah (0.8ubuntu1) quantal; urgency=low15utah (0.8ubuntu1) quantal; urgency=low
1216
1317
=== modified file 'utah/provisioning/ssh.py'
--- utah/provisioning/ssh.py 2013-02-13 15:43:27 +0000
+++ utah/provisioning/ssh.py 2013-03-08 13:53:21 +0000
@@ -26,6 +26,8 @@
2626
27from stat import S_ISDIR27from stat import S_ISDIR
2828
29from jinja2 import Environment, FileSystemLoader
30
29import utah.timeout31import utah.timeout
3032
31from utah import config33from utah import config
@@ -304,6 +306,9 @@
304 if installtype is None:306 if installtype is None:
305 self.installtype = config.installtype307 self.installtype = config.installtype
306308
309 self.template_env = \
310 Environment(loader=FileSystemLoader(config.template_dir))
311
307 def activecheck(self):312 def activecheck(self):
308 """Check if machine is active.313 """Check if machine is active.
309314

Subscribers

People subscribed via source and target branches