Add runbook config and storage management by teferi · Pull Request #11 · seecloud/runbook

teferi · 2016-12-19T11:19:55Z

No description provided.

coveralls · 2016-12-19T11:26:20Z

Coverage remained the same at 0.0% when pulling d6cbcb8 on config_storage into f3303a9 on master.

coveralls · 2016-12-19T12:23:01Z

Coverage remained the same at 0.0% when pulling 1a27e76 on config_storage into f3303a9 on master.

coveralls · 2016-12-19T13:15:12Z

Coverage remained the same at 0.0% when pulling 533aa40 on config_storage into f3303a9 on master.

coveralls · 2016-12-19T17:59:07Z

Coverage remained the same at 0.0% when pulling e2520bf on config_storage into f3303a9 on master.

Configs are split in two: reader and writer for ro/wo api services respectively. main-reader/main-writer entry point files are added.

coveralls · 2016-12-23T12:54:21Z

Coverage remained the same at 0.0% when pulling c513c2f on config_storage into f3303a9 on master.

maretskiy · 2017-01-10T13:34:37Z

runbook/config.py

+
+def get_config(api_type=None):
    """Return cached configuration.



:param api_type: ...

maretskiy · 2017-01-10T13:37:14Z

runbook/config.py

-            config = json.load(open(path))
+            CONF[api_type] = json.load(open(path))
            logging.info("Config is '%s'" % path)
        except IOError as e:


I recommend to catch ValueError as well for cases with invalid JSON

maretskiy · 2017-01-10T13:39:24Z

runbook/config.py

+        jsonschema.validate(CONF[api_type], CONF_SCHEMA)
+    except jsonschema.ValidationError as e:
+        logging.error(e.message)
+        sys.exit(1)


raise, not exit (exiting inside a non-CLI module is a potential bug)

agreed. This whole thing should be rewritten to oss-tools configuration I guess, but it was written well before oss-tools had config management.

maretskiy · 2017-01-10T13:41:25Z

runbook/main_reader.py


 @app.errorhandler(404)
 def not_found(error):
+    logging.error(error)


404 is not an error

that's arguable, but let's change it to debug I guess =)

maretskiy · 2017-01-10T13:51:32Z

runbook/storage.py

+        ensure_index("ms_{}_{}".format("runbooks", region), api_type)
+
+
+if __name__ == "__main__":


This is a strange usage, this module is not a CLI endpoint.

Well in a later commit I think I'm adding a function from the file to setup.cfg Although this does mean that I can do it right here =)

teferi force-pushed the config_storage branch from d6cbcb8 to 1a27e76 Compare December 19, 2016 12:20

teferi force-pushed the config_storage branch from 1a27e76 to 533aa40 Compare December 19, 2016 13:12

teferi force-pushed the config_storage branch from 533aa40 to e2520bf Compare December 19, 2016 17:54

Add runbook config and storage management

c513c2f

Configs are split in two: reader and writer for ro/wo api services respectively. main-reader/main-writer entry point files are added.

teferi force-pushed the config_storage branch from e2520bf to c513c2f Compare December 23, 2016 12:52

maretskiy suggested changes Jan 10, 2017

View reviewed changes


		def get_config(api_type=None):
		"""Return cached configuration.

		ensure_index("ms_{}_{}".format("runbooks", region), api_type)


		if __name__ == "__main__":

Conversation

teferi commented Dec 19, 2016

Uh oh!

coveralls commented Dec 19, 2016

Uh oh!

coveralls commented Dec 19, 2016

Uh oh!

coveralls commented Dec 19, 2016

Uh oh!

coveralls commented Dec 19, 2016

Uh oh!

coveralls commented Dec 23, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants