set log level from env var #3
Conversation
There was a problem hiding this comment.
Thanks @ryandawsonuk!
Overall looks good, just a couple of nits.
After that and a rebase we should be good to go.
handlers.go
Outdated
| returnStatus(w, http.StatusOK, "OK") | ||
| return | ||
| } else { |
There was a problem hiding this comment.
I think this else is not needed.
After this point, you know it's a new session anyway
There was a problem hiding this comment.
The purpose of the else is to log the session struct.
There was a problem hiding this comment.
What I mean is that at that point, you know that this is a new session, because we return inside the if.
Thus, the else is not necessary, you could just log it.
There was a problem hiding this comment.
I understand now. Yes that's clearer to have it in the main flow rather than the else.
handlers.go
Outdated
| @@ -75,6 +78,8 @@ func (s *server) authenticate(w http.ResponseWriter, r *http.Request) { | |||
| logger.Errorf("Failed to save state in store: %v", err) | |||
| returnStatus(w, http.StatusInternalServerError, "Failed to save state in store.") | |||
| return | |||
| } else { | |||
There was a problem hiding this comment.
Same, is else needed here?
There was a problem hiding this comment.
This also logs the session struct and means it can be seen what has changed in since the struct was created.
ryandawsonuk
force-pushed
the
2-loglevels
branch
from
5b5c7c3
to
b2abdda
Compare
to help debug #2
An example of debug logging for a new session:
