Merge lp:~bac/charm-tools/1321316 into lp:charm-tools/1.2

[Brad Crittenden]

Don't die if an option has no type.

If a config option doesn't specify a type, a warning was issued
but the process failed with KeyError. This fix allows it to proceed, using
'string' if not type is given.

This bug affects charmworld ingest, so getting it released soonish would be
great.

https://codereview.appspot.com/98390047/

[Brad Crittenden (bac)]

Reviewers: mp+220286_code.launchpad.net,

Message:
Please take a look.

Description:
Don't die if an option has no type.

If a config option doesn't specify a type, a warning was issued
but the process failed with KeyError. This fix allows it to proceed,
using
'string' if not type is given.

This bug affects charmworld ingest, so getting it released soonish would
be
great.

https://code.launchpad.net/~bac/charm-tools/1321316/+merge/220286

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/98390047/

Affected files (+18, -2 lines):
   A [revision details]
   M charmtools/charms.py
   M tests/test_charm_proof.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: charmtools/charms.py
=== modified file 'charmtools/charms.py'
--- charmtools/charms.py 2014-05-16 15:52:09 +0000
+++ charmtools/charms.py 2014-05-20 14:59:35 +0000
@@ -185,7 +185,7 @@
                  self.warn('config.yaml: option %s has an invalid type (%s)'
                            % (option_name, option_type))
              elif 'default' in option_value:
- expected_type = KNOWN_OPTION_TYPES[option_value['type']]
+ expected_type = KNOWN_OPTION_TYPES[option_type]
                  actual_value = option_value['default']
                  if actual_value is None:
                      notify = (self.info if expected_type in
ALLOW_NONE_DEFAULT
@@ -197,7 +197,7 @@
                      self.err(
                          'config.yaml: type of option %s is specified as '
                          '%s, but the type of the default value is %s'
- % (option_name, option_value['type'],
+ % (option_name, option_type,
                             type(actual_value).__name__))
              else:
                  # Nothing to do: the option type is valid but no default

Index: tests/test_charm_proof.py
=== modified file 'tests/test_charm_proof.py'
--- tests/test_charm_proof.py 2014-05-16 15:52:09 +0000
+++ tests/test_charm_proof.py 2014-05-20 14:59:35 +0000
@@ -71,6 +71,20 @@
          self.linter.check_config_file(self.charm_dir)
          self.assertEqual([], self.linter.lint)

+ def test_missing_type_defaults_to_string(self):
+ # A warning is issued but no failure.
+ self.write_config("""
+ options:
+ string_opt:
+ description: A string option
+ default: some text
+ """)
+ self.linter.check_config_file(self.charm_dir)
+ self.assertEqual(
+ ['W: config.yaml: option string_opt does not have the keys: '
+ 'type'],
+ self.linter.lint)
+
      def test_config_with_invalid_yaml(self):
          self.write_config("""
              options:

[Marco Ceppi (marcoceppi)]

LGTM

https://codereview.appspot.com/98390047/

[Brad Crittenden (bac)]

*** Submitted:

Don't die if an option has no type.

If a config option doesn't specify a type, a warning was issued
but the process failed with KeyError. This fix allows it to proceed,
using
'string' if not type is given.

This bug affects charmworld ingest, so getting it released soonish would
be
great.

R=Marco Ceppi
CC=
https://codereview.appspot.com/98390047

https://codereview.appspot.com/98390047/

=== modified file 'charmtools/charms.py'
--- charmtools/charms.py	2014-05-16 15:52:09 +0000
+++ charmtools/charms.py	2014-05-20 15:08:06 +0000
@@ -185,7 +185,7 @@
                 self.warn('config.yaml: option %s has an invalid type (%s)'
                           % (option_name, option_type))
             elif 'default' in option_value:
-                expected_type = KNOWN_OPTION_TYPES[option_value['type']]
+                expected_type = KNOWN_OPTION_TYPES[option_type]
                 actual_value = option_value['default']
                 if actual_value is None:
                     notify = (self.info if expected_type in ALLOW_NONE_DEFAULT
@@ -197,7 +197,7 @@
                     self.err(
                         'config.yaml: type of option %s is specified as '
                         '%s, but the type of the default value is %s'
-                        % (option_name, option_value['type'],
+                        % (option_name, option_type,
                            type(actual_value).__name__))
             else:
                 # Nothing to do: the option type is valid but no default

=== modified file 'tests/test_charm_proof.py'
--- tests/test_charm_proof.py	2014-05-16 15:52:09 +0000
+++ tests/test_charm_proof.py	2014-05-20 15:08:06 +0000
@@ -71,6 +71,20 @@
         self.linter.check_config_file(self.charm_dir)
         self.assertEqual([], self.linter.lint)
 
+    def test_missing_type_defaults_to_string(self):
+        # A warning is issued but no failure.
+        self.write_config("""
+            options:
+              string_opt:
+                description: A string option
+                default: some text
+            """)
+        self.linter.check_config_file(self.charm_dir)
+        self.assertEqual(
+            ['W: config.yaml: option string_opt does not have the keys: '
+             'type'],
+            self.linter.lint)
+
     def test_config_with_invalid_yaml(self):
         self.write_config("""
             options: