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

fix: enable silent mode #733

Merged
merged 1 commit into from
Nov 15, 2024
Merged

fix: enable silent mode #733

merged 1 commit into from
Nov 15, 2024

Conversation

jmattheis
Copy link
Member

Hides this log output

Failed to find configuration /etc/gotify/config.yml
Failed to find configuration config.yml, using example file config.example.yml

Introduced with the #727

Hides this log output

    Failed to find configuration /etc/gotify/config.yml
    Failed to find configuration config.yml, using example file config.example.yml
@jmattheis jmattheis requested a review from a team as a code owner November 15, 2024 17:41
Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.42%. Comparing base (2eee800) to head (5395112).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #733   +/-   ##
=======================================
  Coverage   79.42%   79.42%           
=======================================
  Files          56       56           
  Lines        2639     2639           
=======================================
  Hits         2096     2096           
  Misses        452      452           
  Partials       91       91           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jmattheis jmattheis merged commit 8639316 into master Nov 15, 2024
4 checks passed
@jmattheis jmattheis deleted the silent branch November 15, 2024 18:16
@eternal-flame-AD
Copy link
Member

eternal-flame-AD commented Nov 15, 2024

I actually intentionally didn't write silent: true... I thought it might not be a good idea someone can just docker run and magically have their server listening on port 80 with admin:admin as the password just because they typoed a config file

I thought this option was originally used because the library produced too many warnings, I think a config fallback warning is useful

@jmattheis
Copy link
Member Author

jmattheis commented Nov 15, 2024

This is only log, the server starts correctly listening on port 80 with admin:admin in docker without any config. And given that gotify/server should be configured via env vars in docker, the error/warning is misleading.

@jmattheis
Copy link
Member Author

Tho, I'm not really against logging stuff like that, just wanted to have the previous behavior for this release as I hadn't noticed this in the review.

@eternal-flame-AD
Copy link
Member

eternal-flame-AD commented Nov 15, 2024

Your call :) IMO logging and warning differences does not count as breaking changes, I just thought it might be confusing or even cause subtle security issues if someone typed config file when running locally, this message clearly states where it expect a config file.

But I agree with you not many people put config in /etc and actually typo it. It might be more likely to happen if they set the workingdir in systemd wrong and missed the config.yml entry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants