Closed msabramo closed 9 years ago
Rebased to fix merge conflict.
@nickstenning, @slafs: What do you guys think of this?
For me this doesn't feel right. I think the responsibility for deciding whether to use rendering with provided full_path or with the builtin path is oddly placed. Don't have much time right now. Let me think about it for a while and I'll get back to you.
I'm not really happy with a flag like this that purports to be general but in practice only works for one export format. Part of the reason for allowing exporters to expose themselves to honcho through setuptools entrypoints is to allow exactly this kind of customisation, but in a more general way.
I appreciate that this is simpler, but if it doesn't generalise it's actually adding complexity to the user interface. In addition, I think you'll find that with #119 merged, it will be much simpler to write custom exporters.
I think I have the same feeling that it's not quite right and could be better. I think maybe the choosing of the template is happening at too low of a level. And the Export classes shouldn't be concerned with it. You should just give them a template and they should render it. After all they are exporters and not template choosers.
Maybe the choice of template should happen further up the stack?
This might intersect a bit with another idea I had - to allow the specified template to be an asset spec (package resource) in addition to a plain ol' absolute file path. This would allow one to pip install honcho-export-supervisord-fancy
and then reference the template with something like -t honcho-export-supervisord-fancy:templates/cool1.jinja2
If that existed, then the default honcho supervisord template could just be a default value that is a resource spec
The problem is that some exporters use multiple templates. Have a look at the upstart exporter to see what I mean.
Ah yes I remember the multiple template thing now. I punted on that. Not sure whether to pass a dict of templates or a directory, which seems more CLI-friendly.
This needs to be reworked in light of the merging of #119.
My current thought is to allow -t
to take a directory full of templates for the exporters that have multiple templates like the upstart exporter...
Thoughts...?
Right now this only works for supervisor; I need to implement upstart support
Or I suppose we could add an error message that says Custom templates not supported for upstart yet
.
Problem with this:
honcho export supervisord out --template=my_supervisord_template.conf.jinja2
will fail because it interprets the template argument as being relative to the honcho package unless it starts with a slash. Workaround: honcho export supervisord out --template=$(pwd)/my_supervisord_template.conf.jinja2
, but I think it's better to change this so honcho-relative paths are assets specs that start with "honcho:" -- e.g.: "honcho:supervisord/supervisord.conf"
Or add "."
to the filesystem search path.
Apparently, foreman
uses a directory for templates, although this wasn't at all obvious from help or docs:
Do you want to rework this so the templates directory is configurable?
@nickstenning: Just curious why you closed this? I haven't gotten around to updating this yet, but I wonder if you closed it, because you don't think it's a good idea? Are you open to another PR?
No, it's a fine idea. Feel free to open a PR when you get a chance to update the code.
Ok, cool, just checking before I spend the time. Thanks!
OK, I just updated this so that --template
is a directory. I'm not quite sure how to test this in an automated way, but I did do manual testing.
[marca@marca-mac2 export_test]$ ls -l upstart_template.d
total 24
-rw-r--r--+ 1 marca staff 353 Feb 23 06:01 master.conf
-rw-r--r--+ 1 marca staff 380 Feb 23 06:02 process.conf
-rw-r--r--+ 1 marca staff 119 Feb 23 06:02 process_master.conf
[marca@marca-mac2 export_test]$ honcho export upstart upstart-export --template upstart_template.d
2015-02-23 07:59:43 [20837] [INFO] Writing 'upstart-export/export_test.conf'
2015-02-23 07:59:43 [20837] [INFO] Writing 'upstart-export/export_test-ansvc.conf'
2015-02-23 07:59:43 [20837] [INFO] Writing 'upstart-export/export_test-ansvc-1.conf'
[marca@marca-mac2 export_test]$ honcho export upstart upstart-export
2015-02-23 08:00:16 [20841] [INFO] Writing 'upstart-export/export_test.conf'
2015-02-23 08:00:16 [20841] [INFO] Writing 'upstart-export/export_test-ansvc.conf'
2015-02-23 08:00:16 [20841] [INFO] Writing 'upstart-export/export_test-ansvc-1.conf'
[marca@marca-mac2 export_test]$ ls -l supervisor-template.d
total 8
-rw-r--r--+ 1 marca staff 776 Dec 23 08:09 supervisord.conf
[marca@marca-mac2 export_test]$ honcho export supervisord out --template=supervisor-template.d
2015-02-23 08:00:39 [20847] [INFO] Writing 'out/export_test.conf'
[marca@marca-mac2 export_test]$ honcho export supervisord out
2015-02-23 08:01:05 [20855] [INFO] Writing 'out/export_test.conf'
This adds the ability to use a custom template when exporting to supervisord.
Fixes: GH-88
Example: