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

13817 improve logs #479

Merged
merged 6 commits into from
Aug 23, 2018
Merged

13817 improve logs #479

merged 6 commits into from
Aug 23, 2018

Conversation

thatandromeda
Copy link
Contributor

Ready for merge?

YES

What does this PR do?

Adds lograge gem and a log formatter to make logs more readable. Writes API tokens to logs.

Helpful background context (if appropriate)

How can a reviewer manually see the effects of these changes?

Run the app locally with production env and observe log/production.log.

What are the relevant tickets?

Screenshots (if appropriate)

Todo:

  • [ ] Tests
  • [ ] Documentation
  • [ ] Stakeholder approval

Requires Database Migrations?

NO

Includes new or updated dependencies?

YES

@coveralls
Copy link

coveralls commented Aug 20, 2018

Pull Request Test Coverage Report for Build 522

  • 27 of 27 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.226%

Totals Coverage Status
Change from base Build 520: 0.0%
Covered Lines: 1677
Relevant Lines: 2015

💛 - Coveralls

This should substantially clean up the log files; its defaults match
the desired behavior (modulo next commit).
Alphabetizing it causes tests to fail (!), presumably due to a
poorly expressed dependency somewhere in one of these gems. (Gems are
`require`d in the order listed.) Tracking down where the dependency is
would be a pain.
end
end

def authenticate_user_from_token!
user = authentication_token && User.find_by_authentication_token(authentication_token.to_s)
Rails.logger.info "Attempted login from token #{authentication_token.to_s}"
Copy link
Member

Choose a reason for hiding this comment

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

love it

@@ -74,4 +74,7 @@
config.action_mailer.default_url_options = {
host: (ENV['EMAIL_DOMAIN'] || 'chillingeffects.org')
}
config.action_mailer.default_url_options = { host: (ENV['EMAIL_DOMAIN'] || 'chillingeffects.org') }
Copy link
Member

Choose a reason for hiding this comment

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

these should be updated to lumendatabase.org but perhaps a more through audit should be made as a separate task to fix them all at once

@@ -2,11 +2,11 @@
require 'devise/orm/active_record'

config.secret_key = Rails.application.secrets.devise_secret
config.mailer_sender = "admin@chillingeffects.org"
config.mailer_sender = 'admin@chillingeffects.org'
Copy link
Member

Choose a reason for hiding this comment

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

ditto to previous email comment

@thatandromeda thatandromeda merged commit 2e8e8f9 into dev Aug 23, 2018
@thatandromeda thatandromeda deleted the 13817_improve_logs branch August 23, 2018 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants