Skip to content
This repository has been archived by the owner on Sep 16, 2022. It is now read-only.

Fix Django logging #154

Merged
merged 2 commits into from
May 14, 2019

Conversation

rptrchv
Copy link
Contributor

@rptrchv rptrchv commented May 14, 2019

Fixes #136
Done:

  • setup Sentry support
  • enabled it for prod and stage envs (only if SENTRY_DSN environment variable is available)
  • enabled errors output to console for all environments

Copy link
Contributor

@a-martynovich a-martynovich left a comment

Choose a reason for hiding this comment

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

I believe you could avoid duplicating code in prod.py and stage.py by moving it to base.py. Since it is executed only if SENTRY_DSN is set, one must know what he's doing if he's setting it in dev environment.

@rptrchv
Copy link
Contributor Author

rptrchv commented May 14, 2019

I believe you could avoid duplicating code in prod.py and stage.py by moving it to base.py. Since it is executed only if SENTRY_DSN is set, one must know what he's doing if he's setting it in dev environment.

IMO we shouldn't manage the project behavior by setting/unsetting any environment vars. Instead we must explicitly set this in a settings file. As for environment vars - they are only for storage some values, not for management. @vpetersson wdyt?

@vpetersson
Copy link
Contributor

I have no strong opinion either way. It might be a bit more DRY to use base.py but sometimes explicit is better than implicit.

That said, I disagree slightly about your point about env vars @rptrchv. They are fairly fundamental to 12 factor apps

@a-martynovich
Copy link
Contributor

I'm strongly against code duplication (even though I may allow it myself sometimes).
Also I'm against modifying a VCS-tracked file for local development needs. This leads to accidentaly committed changes, rebase/pull troubles (a need of stash/unstash every time), etc. If something should be modified locally (or generated) it shouldn't be tracked.

@vpetersson
Copy link
Contributor

@a-martynovich I'm fully with you with regards to modifying VCS tracked files, but that isn't necessarily the case here. You can either modify the dev "profile" to not use Sentry, or modify both the production and staging profile to use it.

Roman P added 2 commits May 14, 2019 18:54
Fixes #136
Done:
 - setup Sentry support
 - enabled it for prod and stage envs (only if SENTRY_DSN environment variable is available)
 - enabled errors output to console for all environments
@rptrchv
Copy link
Contributor Author

rptrchv commented May 14, 2019

@a-martynovich Got rid of that duplicated code

@a-martynovich
Copy link
Contributor

well I'm happy with that.
LGTM

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

Successfully merging this pull request may close these issues.

Fix Django logging
3 participants