Repository navigation
Ximalas syslog facility - #115
Conversation
|
Thanks for your contribution! |
Ximalas
left a comment
There was a problem hiding this comment.
It's an open question whether to map the string value from the config file to the corresponding syslog.Priority value in parseSyslogOutlet() or in WriteEntry().
| case "local5": fac = syslog.LOG_LOCAL5 | ||
| case "local6": fac = syslog.LOG_LOCAL6 | ||
| case "local7": fac = syslog.LOG_LOCAL7 | ||
| } |
There was a problem hiding this comment.
this should be done in config.go parseSyslogOutlet, the SyslogOutlet.Facility member should be syslog.Priority ( https://golang.org/pkg/log/syslog/#Priority )
|
I'm not sure if I understood you correctly. I can create |
problame
left a comment
There was a problem hiding this comment.
Thanks for following up so quickly!
Now the last bit that is missing is some unit test coverage (see config/*_test.go): I would like to see a check that the default value is indeed syslog.LOG_LOCAL0 and another check that asserts that facility can be overridden by specifying it in the yaml.
FunctionTestRPC in confg_rpc_test.go might serve as as inspiration, although I don't expect all facility names to be tested.
| - type: syslog | ||
| level: info | ||
| format: human | ||
| facility: kern |
There was a problem hiding this comment.
please use go string templating ( https://golang.org/pkg/text/template/ ) or similiar to avoid the repetition here
| case "local7": | ||
| out.Facility = syslog.LOG_LOCAL7 | ||
| default: | ||
| out.Facility = syslog.LOG_LOCAL0 |
There was a problem hiding this comment.
should be fmt.Errorf("unknown facility name %q", in.Facility)
|
|
||
| type SyslogLoggingOutlet struct { | ||
| LoggingOutletCommon `yaml:",inline"` | ||
| Facility string `yaml:"facility,default=local0"` |
There was a problem hiding this comment.
hm, sorry, maybe I was unclear: I would like to see the syslog.Facility here. that should be possible by defining a wrapper type and implementing UnmarshalYAML for it.
|
I'm in unknown waters, that's for sure. I'm stuck at trying to convert --- FAIL: TestOutletTypes (0.00s)
Error Trace: config_test.go:42
config_global_test.go:27
config_global_test.go:31
Error: Received unexpected error yaml: unmarshal errors:
line 1: cannot unmarshal !!str `local0` into syslog.Priority
--- FAIL: TestSyslogLoggingOutletFacility (0.00s)
Error Trace: config_test.go:42
config_global_test.go:27
config_global_test.go:100
Error: Received unexpected error yaml: unmarshal errors:
line 7: cannot unmarshal !!str `kern` into syslog.Priority
I had this wonderful idea about creating |
|
I fixed the config parsing errors in a private branch. edit: nvmd, using the http remote worked (see force push) |
69da2be to
9b1a216
Compare
6b11f40 to
0de283b
Compare
0de283b to
a0f301d
Compare
|
On Fri, 15 Mar 2019 09:48-0700, Christian Schwarz wrote:
I fixed the config parsing errors in a private branch.
That's good. I'm sorry for being absent these past weeks.
Please give me write permissions on your PR branch, so that we can
continue work within this PR:
https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork
Hum, I'm unable to locate my PR in zrepl/zrepl to complete your
request. Can I assume I don't need to do any action?
…--
----------------------------------------------------------------------
Trond Endrestøl | Trond.Endrestol@ximalas.info
ACM, NAS, NUUG, USENIX | FreeBSD 12.0-S & Alpine 2.21.9999
|
|
No more action required, except for testing the binary build if possible: |
Here's my contribution to issue #60, adding configurable syslog facility to zrepl.