Merge lp:~tvansteenburgh/charm-tools/no-symlinks into lp:charm-tools/1.3

Proposed by Tim Van Steenburgh
Status: Needs review
Proposed branch: lp:~tvansteenburgh/charm-tools/no-symlinks
Merge into: lp:charm-tools/1.3
Diff against target: 207 lines (+18/-106)
7 files modified
charmtools/charms.py (+1/-1)
charmtools/generators/template.py (+8/-0)
charmtools/templates/python/config.yaml (+0/-5)
charmtools/templates/python/files/hooks_symlinked/hooks.py (+0/-54)
charmtools/templates/python/template.py (+0/-16)
tests/test_charm_proof.py (+4/-1)
tests_functional/create/test_python_create.py (+5/-29)
To merge this branch: bzr merge lp:~tvansteenburgh/charm-tools/no-symlinks
Reviewer Review Type Date Requested Status
Marco Ceppi (community) Approve
Review via email: mp+233395@code.launchpad.net

Description of the change

Removes option to created symlinked hooks when creating a new charm, since symlinks don't work on Windows.

Run all tests with `make check`. Test manually with:

. bin/activate
charm create foo /tmp

To post a comment you must log in.
Revision history for this message
Marco Ceppi (marcoceppi) :
review: Needs Fixing
Revision history for this message
Marco Ceppi (marcoceppi) :
review: Abstain
Revision history for this message
Marco Ceppi (marcoceppi) wrote :

LGTM

review: Approve

Unmerged revisions

339. By Tim Van Steenburgh

Remove option to symlink hooks when creating charm

338. By Tim Van Steenburgh

Fix charm-proof email validation

formataddr() will put quotes around a name component that
contains special chars. We need to remove those to accurately
compare against the original maintainer string.

Also fixes tests that were broken due to new files in
charmhelpers.core

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'charmtools/charms.py'
--- charmtools/charms.py 2014-06-17 15:44:04 +0000
+++ charmtools/charms.py 2014-09-04 17:14:02 +0000
@@ -453,7 +453,7 @@
453 for maintainer in maintainers:453 for maintainer in maintainers:
454 (name, address) = email.utils.parseaddr(maintainer)454 (name, address) = email.utils.parseaddr(maintainer)
455 formatted = email.utils.formataddr((name, address))455 formatted = email.utils.formataddr((name, address))
456 if formatted != maintainer:456 if formatted.replace('"', '') != maintainer:
457 linter.warn(457 linter.warn(
458 'Maintainer format should be "Name <Email>", '458 'Maintainer format should be "Name <Email>", '
459 'not "%s"' % formatted)459 'not "%s"' % formatted)
460460
=== modified file 'charmtools/generators/template.py'
--- charmtools/generators/template.py 2014-07-29 18:15:41 +0000
+++ charmtools/generators/template.py 2014-09-04 17:14:02 +0000
@@ -46,6 +46,14 @@
46 """Return default configuration for this template, loaded from a46 """Return default configuration for this template, loaded from a
47 config.yaml file.47 config.yaml file.
4848
49 This is a sample config.yaml that configures one user prompt::
50
51 prompts:
52 symlink:
53 prompt: Symlink all hooks to one python source file? [yN]
54 default: n
55 type: boolean
56
49 """57 """
50 path = self.config_path()58 path = self.config_path()
51 if os.path.exists(path):59 if os.path.exists(path):
5260
=== removed file 'charmtools/templates/python/config.yaml'
--- charmtools/templates/python/config.yaml 2014-05-27 21:23:41 +0000
+++ charmtools/templates/python/config.yaml 1970-01-01 00:00:00 +0000
@@ -1,5 +0,0 @@
1prompts:
2 symlink:
3 prompt: Symlink all hooks to one python source file? [yN]
4 default: n
5 type: boolean
60
=== removed directory 'charmtools/templates/python/files/hooks_symlinked'
=== removed file 'charmtools/templates/python/files/hooks_symlinked/hooks.py'
--- charmtools/templates/python/files/hooks_symlinked/hooks.py 2014-05-27 21:23:41 +0000
+++ charmtools/templates/python/files/hooks_symlinked/hooks.py 1970-01-01 00:00:00 +0000
@@ -1,54 +0,0 @@
1#!/usr/bin/python
2
3import os
4import sys
5
6sys.path.insert(0, os.path.join(os.environ['CHARM_DIR'], 'lib'))
7
8from charmhelpers.core import (
9 hookenv,
10 host,
11)
12
13hooks = hookenv.Hooks()
14log = hookenv.log
15
16SERVICE = '$metadata.package'
17
18
19@hooks.hook('install')
20def install():
21 log('Installing $metadata.package')
22
23
24@hooks.hook('config-changed')
25def config_changed():
26 config = hookenv.config()
27
28 for key in config:
29 if config.changed(key):
30 log("config['{}'] changed from {} to {}".format(
31 key, config.previous(key), config[key]))
32
33 config.save()
34 start()
35
36
37@hooks.hook('upgrade-charm')
38def upgrade_charm():
39 log('Upgrading $metadata.package')
40
41
42@hooks.hook('start')
43def start():
44 host.service_restart(SERVICE) or host.service_start(SERVICE)
45
46
47@hooks.hook('stop')
48def stop():
49 host.service_stop(SERVICE)
50
51
52if __name__ == "__main__":
53 # execute a hook based on the name the program is called by
54 hooks.execute(sys.argv)
550
=== modified file 'charmtools/templates/python/template.py'
--- charmtools/templates/python/template.py 2014-07-29 18:15:41 +0000
+++ charmtools/templates/python/template.py 2014-09-04 17:14:02 +0000
@@ -46,7 +46,6 @@
4646
47 self._template_file(config, path.join(root, outfile))47 self._template_file(config, path.join(root, outfile))
4848
49 self._cleanup_hooks(config, output_dir)
50 self._install_charmhelpers(output_dir)49 self._install_charmhelpers(output_dir)
5150
52 def _copy_files(self, output_dir):51 def _copy_files(self, output_dir):
@@ -72,21 +71,6 @@
72 os.rename(o.name, outfile)71 os.rename(o.name, outfile)
73 os.unlink(backupname)72 os.unlink(backupname)
7473
75 def _cleanup_hooks(self, config, output_dir):
76 rmdir = 'hooks' if config['symlink'] else 'hooks_symlinked'
77 shutil.rmtree(os.path.join(output_dir, rmdir))
78
79 if config['symlink']:
80 os.rename(
81 os.path.join(output_dir, 'hooks_symlinked'),
82 os.path.join(output_dir, 'hooks')
83 )
84 # Symlinks must be relative so that they are copied properly
85 # when output_dir is moved to it's final location.
86 for link in ['config-changed', 'install', 'start', 'stop',
87 'upgrade-charm']:
88 os.symlink('hooks.py', os.path.join(output_dir, 'hooks', link))
89
90 def _install_charmhelpers(self, output_dir):74 def _install_charmhelpers(self, output_dir):
91 helpers_dest = os.path.join(output_dir, 'lib', 'charmhelpers')75 helpers_dest = os.path.join(output_dir, 'lib', 'charmhelpers')
92 if not os.path.exists(helpers_dest):76 if not os.path.exists(helpers_dest):
9377
=== modified file 'tests/test_charm_proof.py'
--- tests/test_charm_proof.py 2014-06-17 15:44:04 +0000
+++ tests/test_charm_proof.py 2014-09-04 17:14:02 +0000
@@ -420,7 +420,10 @@
420 """Maintainers field happy path."""420 """Maintainers field happy path."""
421 linter = Mock()421 linter = Mock()
422 charm = {422 charm = {
423 'maintainers': ['Tester <tester@example.com>'],423 'maintainers': [
424 'Tester <tester@example.com>',
425 'Tester Joe H. <tester@example.com>',
426 ]
424 }427 }
425 validate_maintainer(charm, linter)428 validate_maintainer(charm, linter)
426 self.assertFalse(linter.err.called)429 self.assertFalse(linter.err.called)
427430
=== modified file 'tests_functional/create/test_python_create.py'
--- tests_functional/create/test_python_create.py 2014-06-20 19:46:59 +0000
+++ tests_functional/create/test_python_create.py 2014-09-04 17:14:02 +0000
@@ -45,46 +45,22 @@
45 def tearDown(self):45 def tearDown(self):
46 shutil.rmtree(self.tempdir)46 shutil.rmtree(self.tempdir)
4747
48 def _expected_files(self, symlinked=False):48 def _expected_files(self):
49 static_files = list(flatten(pkg_resources.resource_filename(49 static_files = list(flatten(pkg_resources.resource_filename(
50 'charmtools', 'templates/python/files')))50 'charmtools', 'templates/python/files')))
51 static_files = [f for f in static_files
52 if not f.startswith('hooks_symlinked/')]
53 if symlinked:
54 static_files.append('hooks/hooks.py')
55 dynamic_files = [51 dynamic_files = [
56 'lib/charmhelpers/__init__.py',52 'lib/charmhelpers/__init__.py',
57 'lib/charmhelpers/core/__init__.py',53 'lib/charmhelpers/core/__init__.py',
58 'lib/charmhelpers/core/fstab.py',54 'lib/charmhelpers/core/fstab.py',
59 'lib/charmhelpers/core/hookenv.py',55 'lib/charmhelpers/core/hookenv.py',
60 'lib/charmhelpers/core/host.py',56 'lib/charmhelpers/core/host.py',
57 'lib/charmhelpers/core/services/__init__.py',
58 'lib/charmhelpers/core/services/base.py',
59 'lib/charmhelpers/core/services/helpers.py',
60 'lib/charmhelpers/core/templating.py',
61 ]61 ]
62 return sorted(static_files + dynamic_files)62 return sorted(static_files + dynamic_files)
6363
64 @patch('__builtin__.raw_input')
65 @patch('charmtools.create.setup_parser')
66 def test_interactive(self, setup_parser, raw_input_):
67 """Functional test of a full 'charm create' run."""
68 class args(object):
69 charmname = 'testcharm'
70 charmhome = self.tempdir
71 template = 'python'
72 accept_defaults = False
73 verbose = False
74
75 setup_parser.return_value.parse_args.return_value = args
76 raw_input_.side_effect = ['Y']
77
78 main()
79
80 outputdir = join(self.tempdir, args.charmname)
81 actual_files = sorted(flatten(outputdir))
82 expected_files = self._expected_files(symlinked=True)
83 metadata = yaml.load(open(join(outputdir, 'metadata.yaml'), 'r'))
84
85 self.assertEqual(expected_files, actual_files)
86 self.assertEqual(metadata['name'], args.charmname)
87
88 @patch('charmtools.create.setup_parser')64 @patch('charmtools.create.setup_parser')
89 def test_defaults(self, setup_parser):65 def test_defaults(self, setup_parser):
90 """Functional test of a full 'charm create' run."""66 """Functional test of a full 'charm create' run."""

Subscribers

People subscribed via source and target branches