tierpod / dmarc-report-converter

Convert dmarc reports from xml to human-readable formats
MIT License
237 stars 25 forks source link

Templates should just be part of the compiled binary, ideally. #20

Closed morrowc closed 2 years ago

morrowc commented 2 years ago

Having the templates read at run-time means that the current working directory when launching the binary must have a `./templates/...' with the appropriate content included. If this isn't the case you get:

$ /usr/local/bin/dmarc-report-converter  -config /etc/dmarc-reporting/config.yaml
panic: open ./templates/html_static.gotmpl: no such file or directory

goroutine 1 [running]:
main.loadTemplate(0x8c1c680, 0x1e, 0xc)
    /usr/local/bin/config.go:63 +0xf6
main.loadConfig(0xbff2ec2a, 0x43, 0x2, 0x3, 0x0)
    /usr/local/bin/config.go:90 +0x11c
main.main()
    /usr/local/bin/main.go:30 +0x159
$ 

how about discussion-over-code-review? :)

tierpod commented 2 years ago

@morrowc Thank you for your idea and PR! Sorry for late answer.

I see that searching templates in current working directory is a bad idea.

Your way has many advantages for end user:

And one disadvantage:

Maybe we can keep your changes and add another output format for using external template file? What do you think? Something like this:

output:
  format: "external_template"
  external_template: "/path/to/file.gotmpl"
morrowc commented 2 years ago

howdy! no worries on the delays...

I think there are 2 things going on: 1) an assumption about directory for deployment/runtime 2) a normal form/style change to include the template content at compile time instead of rationalizing how to do 1 better.

You mention: "easier for development", which seems specious to me, since unless you included a ton of random/extra data in the template data struct, you'd always have to reompile to get data included and output that in the template(s) output? (unless you mean just changing the free-text... which seems like a fairly light load of trade-off to me).

tierpod commented 2 years ago

unless you mean just changing the free-text

Yes, I meant this. For me, data struct is almost static and isn't changed a lot. But html or txt templates can be changed a lot during development process or even in production usage. I don't know, maybe someone already uses self-written custom templates. That's why I would like to keep ability to use external template via another output format.

Anyway, your PR is good, I'm going to merge it soon.

morrowc commented 2 years ago

ah! ok, I see your usecase now. If you merge this and send me an issue I'll put in a pr that we can discuss?

tierpod commented 2 years ago

I merged your PR and added external_template output format. Thanks!

tierpod commented 2 years ago

New version v0.6 was released