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

Log when deprecated and new args are set at the same time #1243

Merged
merged 2 commits into from
Jan 30, 2022

Conversation

paskal
Copy link
Sponsor Collaborator

@paskal paskal commented Jan 30, 2022

For example, when notify.telegram.token and telegram.token are both set but to different values, user might see "access denied" error in log on attempt to send telegram notification, thinking that notify.telegram.token value is used, when in fact it is ignored and only telegram.token is used.

New behavior is the same, ignoring the old param when new one is set, but issuing the error log message which explicitly tells the user about that.

Resolves #1218.

backend/app/cmd/cmd.go Outdated Show resolved Hide resolved
backend/app/cmd/server.go Show resolved Hide resolved
For example, when notify.telegram.token and telegram.token
are both set but to different values, user might see
"access denied" error in log on attempt to send telegram
notification, thinking that notify.telegram.token value
is used, when in fact it is ignored and only telegram.token
is used.

New behavior is the same, ignoring the old param when new
one is set, but issuing the error log message which
explicitly tells the user about that.

Resolves #1218.
Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@paskal paskal merged commit 31af19e into master Jan 30, 2022
@paskal paskal deleted the paskal/deprecated_params branch January 30, 2022 21:40
@paskal paskal added this to the v1.9 milestone Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notify admin not work: deprecated params are not used when new ones are set
2 participants