search

davidmintz / court-interpreters-office

web application for managing a busy court interpreters office.
MIT License
6 stars 2 forks source link

Remove DB Logging #92

Open TomHAnderson opened 4 years ago

TomHAnderson commented 4 years ago

Logging by its nature cannot be done inside a database because if the database is down how can a log be created?

The application errors if the database logging table does not exist so it tries to create a log in the database. That's how this functions today.

davidmintz commented 4 years ago

I'm a bit confused. If we can't (or shouldn't) write log entries to a database, why does https://docs.laminas.dev/laminas-log/writers/#writing-to-databases exist?

The way it's set now, there's both a file writer and a database writer. If the first one fails, in my experience, the application stops with a fatal error. I have found the latter one enormously helpful for looking at what's been happening with the benefit of SQL.

TomHAnderson commented 4 years ago

You should never use an ERROR log in a database. But say you wanted to log events which occur as a result of processing database entries then logging to the database is ok. But an ERROR log in a database cannot log an error about the database to the database.

davidmintz commented 4 years ago

Forgive me for being obtuse. If there's a database error it gets logged to the log file. If it's any other kind of error I find it extremely helpful to read about it in my app_event_log table. I don't see the harm in logging non -database-related error messages to the database.

David Mintz https://davidmintz.org Capitalism has failed. https://wsws.org

On Tue, Jun 30, 2020, at 5:22 PM, Tom H Anderson wrote:

You should never use an ERROR log in a database. But say you wanted to log events which occur as a result of processing database entries then logging to the database is ok. But an ERROR log in a database cannot log an error about the database to the database.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/davidmintz/court-interpreters-office/issues/92#issuecomment-652051864, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACLFCF6CFUEJYAPSUTEWPOLRZJJRFANCNFSM4OMTUD2Q.

TomHAnderson commented 4 years ago

Here's an exercise for you:

  1. delete your database then run 
php public/index.php

You will receive an error.

  1. create the database but do not create any tables then run 
    php public/index.php

You will see a log error message because the app_event_log table does not exist and the log writer for the db is loaded in a route listener and the app is trying to log the database error to the database.

In the PR I made today I removed the event listener for the route and added the db logger to the log factory. I also changed the development local.php to use a custom log factory which does not include the db logger. This can of course be changed on a per-install basis in the local.php and overriding of the log factory may not be needed once the migrations are under control.

Please review the log changes I made in today's PR.