cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

I like this.
I had a few comments inline.

One thing i worry about is bytes versus string.
It seems like our binary-safe json handler has the effect that sometimes a value is a string and sometimes it is bytes depending on the content that was in it.

>From a consumer perspective, that is annoying.
I'd rather know that a given path is *always* bytes or *always* a string.

Otherwise you get code like in cloud-init's decode_binary() that decodes to string if not a string. I think it just kind of results in sloppy code.  I'm bit by having lots of places in cloud-init where we accidently got by without failure in python2 and the most of the time get by in python3.

thoughts?


Diff comments:

> diff --git a/cloudinit/handlers/jinja_template.py b/cloudinit/handlers/jinja_template.py
> new file mode 100644
> index 0000000..d29a107
> --- /dev/null
> +++ b/cloudinit/handlers/jinja_template.py
> @@ -0,0 +1,86 @@
> +# Copyright (C) 2012 Canonical Ltd.
> +# Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> +# Copyright (C) 2012 Yahoo! Inc.

new file, drop the copied Copyright and Author, just go with the shortest header possible.

> +#
> +# Author: Scott Moser <scott.moser@xxxxxxxxxxxxx>
> +# Author: Juerg Haefliger <juerg.haefliger@xxxxxx>
> +# Author: Joshua Harlow <harlowja@xxxxxxxxxxxxx>
> +#
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +import os
> +
> +from cloudinit import handlers
> +from cloudinit import log as logging
> +from cloudinit.sources import INSTANCE_JSON_FILE
> +from cloudinit.templater import render_string
> +from cloudinit.util import b64d, load_file, load_json
> +
> +from cloudinit.settings import PER_ALWAYS
> +
> +LOG = logging.getLogger(__name__)
> +
> +
> +class JinjaTemplatePartHandler(handlers.Handler):
> +
> +    prefixes = ['## template: jinja']
> +
> +    def __init__(self, paths, **_kwargs):
> +        handlers.Handler.__init__(self, PER_ALWAYS, version=3)
> +        self.paths = paths
> +        self.sub_handlers = {}
> +        for handler in _kwargs.get('sub_handlers', []):
> +            for ctype in handler.list_types():
> +                self.sub_handlers[ctype] = handler
> +
> +    def handle_part(self, data, ctype, filename, payload, frequency, headers):
> +        if ctype in handlers.CONTENT_SIGNALS:
> +            return
> +        instance_data = {}
> +        json_file_path = os.path.join(self.paths.run_dir, INSTANCE_JSON_FILE)
> +        if os.path.exists(json_file_path):
> +            instance_data = load_json(load_file(json_file_path))
> +        else:
> +            LOG.warning(
> +                'Instance data not yet present at {0}'.format(json_file_path))

how real of a case is this? I dont thin it should happen.
or will it *always* happen?

> +        instance_jinja_vars = convert_jinja_instance_data(instance_data)
> +        rendered_payload = render_string(payload, instance_jinja_vars)
> +        rendered_payload = rendered_payload.replace(
> +            self.prefixes[0] + '\n', '')
> +        subtype = handlers.type_from_starts_with(rendered_payload)
> +        sub_handler = self.sub_handlers.get(subtype)
> +        if not sub_handler:
> +            LOG.warning(
> +                'Ignoring jinja template for {0}. Could not find supported'
> +                ' sub-handler for type {1}'.format(
> +                    filename, subtype))
> +            return
> +        if sub_handler.handler_version == 3:
> +            sub_handler.handle_part(
> +                data, ctype, filename, rendered_payload, frequency, headers)
> +        elif sub_handler.handler_version == 2:
> +            sub_handler.handle_part(
> +                data, ctype, filename, rendered_payload, frequency)
> +
> +
> +def convert_jinja_instance_data(d, prefix='', sep='/', decode_paths=()):
> +    """Process instance-data.json dict for use in jinja templates.
> +

i think we might as well just store the '_' versions of keys.
no?

> +    Replace hyphens with underscores for jinja templates and decode any
> +    base64-encoded-keys.
> +    """
> +    result = {}
> +    for key, value in d.items():
> +        key_path = '{0}{1}{2}'.format(prefix, sep, key) if prefix else key
> +        if key_path in decode_paths:
> +            value = b64d(value)
> +        key = key.replace('-', '_')
> +        if isinstance(value, dict):
> +            result[key] = convert_jinja_instance_data(
> +                value, key_path, sep=sep, decode_paths=decode_paths)
> +        else:
> +            result[key] = value
> +    return result
> +
> +
> +# vi: ts=4 expandtab


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/334030
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:parameterize-cloud-config into cloud-init:master.