Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syslog-ident config parameter in Valkey #301

Closed
hwware opened this issue Apr 11, 2024 · 3 comments · Fixed by #390
Closed

syslog-ident config parameter in Valkey #301

hwware opened this issue Apr 11, 2024 · 3 comments · Fixed by #390

Comments

@hwware
Copy link
Member

hwware commented Apr 11, 2024

In valkey.conf and sentinel.conf, there are 2 parameters:

syslog-ident redis

syslog-ident sentinel

Do we need update syslog-ident redis to syslog-ident valkey or just keep it?

Thanks

@PingXie
Copy link
Member

PingXie commented Apr 11, 2024

sentinel looks fine to me.

I think it makes sense to default to "redis" for valkey-server's syslog-ident.

Again, IMO, the ideal experience is that without the compat switch (separate discussion on run-time vs build-time), we default to valkey for all identities; and then there is this single compat switch, when enabled, we get full compat with redis, for both application developers and operators/admins. over time, the community will add more native support for Valkey, and there will be a tipping point when the majority of users can run the image by default just fine without the compat layer.

@zuiderkwast
Copy link
Contributor

zuiderkwast commented Apr 18, 2024

I think we shall change it. The question is whether we need the extended compatibility config for it or if we can just change it.

Syslog is disabled by default.

The syslog-ident can be configured. "redis" is just a default value.

Relevant lines:

src/config.c:    createStringConfig("syslog-ident", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.syslog_ident, "redis", NULL, NULL),
valkey.conf:# syslog-ident redis

IMO, we can just change it. A changed config default is a breaking change though, so it should be mentioned in the release notes.

@karthyuom
Copy link
Contributor

@zuiderkwast, I have raised the PR for the above, please review. Thanks.

zuiderkwast pushed a commit that referenced this issue Apr 30, 2024
Default value for the "syslog-ident" config changed from "redis" to
"valkey".

Fixes #301.

---------

Signed-off-by: Karthick Ariyaratnam <karthyuom@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants