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

Chore: Respect Logging Pref of Parent/Supervisor App #42

Open
3 tasks
nelsonic opened this issue Dec 3, 2022 · 0 comments
Open
3 tasks

Chore: Respect Logging Pref of Parent/Supervisor App #42

nelsonic opened this issue Dec 3, 2022 · 0 comments
Labels
chore a tedious but necessary task often paying technical debt discuss Share your constructive thoughts on how to make progress with this issue priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished T1h Time Estimate 1 Hour

Comments

@nelsonic
Copy link
Member

nelsonic commented Dec 3, 2022

As noted by @vans163 in #36 (comment) our current approach to logging
is a little crude and could result in a lot of noise and potentially even perf issues on a high traffic app. 💭

As highlighted in https://github.com/dwyl/envar/pull/36/files#diff-c24eb47ea99dc64a5109913f247d10fd83fa57e93cd9b4609cb703552215c901 there are 4 instances of Logger calls:

envar/lib/envar.ex

Lines 31 to 33 in 03b035d

if is_nil(val) do
Logger.error("ERROR: #{varname} Environment Variable is not set")
end

envar/lib/envar.ex

Lines 55 to 58 in 03b035d

case System.get_env(varname) do
nil ->
Logger.debug("#{varname} Environment Variable is not set")
false

envar/lib/envar.ex

Lines 165 to 170 in 03b035d

if File.exists?(path) do
load(filename)
else
Logger.error("Required .env file does not exist at path: #{path}")
:error
end

envar/lib/envar.ex

Lines 199 to 202 in 03b035d

def read(filename) do
path = Path.join(File.cwd!(), filename)
Logger.debug(".env file path: #{path}")

Most of these are conditional, i.e. they won't log if the variable is set or file exists.
But this last one ... I have to agree with @vans163 is sloppy. 🤦‍♂️

Todo

  • Re-introduce Logs into the project but they should:
    • Be OFF by default i.e. not noisy! [they can be enabled on :dev or :test]
    • Respect the Logging level of the "parent" (e.g. Phoenix) App.

Side Quest:

@nelsonic nelsonic added chore a tedious but necessary task often paying technical debt discuss Share your constructive thoughts on how to make progress with this issue T1h Time Estimate 1 Hour priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished labels Dec 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore a tedious but necessary task often paying technical debt discuss Share your constructive thoughts on how to make progress with this issue priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished T1h Time Estimate 1 Hour
Projects
None yet
Development

No branches or pull requests

1 participant