Skip to content

Combine log entries (Prereq for OC-1541)

Boros Gábor requested to merge combine-log-entries into master

Created by: bradenmacdonald

This change combines GeneralLogEntry, InstanceLogEntry, ServerLogEntry into a single LogEntry class that uses the contenttypes framework to hold a reference to the object that "owns" each log entry.

This change is required in order to accommodate the refactoring in #63. Specifically, there is a new intermediate class, AppServer, which would need a new table to hold its logs. And AppServer is abstract (since multi-table inheritance in django gets very messy), so there are potentially a number of concrete AppServer subclasses that we may have, and we can't expect to create a separate logging table for each one. Luckily, django already has a wonderful and longstanding solution to this problem, which is the contenttypes framework.

Pros:

  • Code is way simpler
  • Loading logs way faster/simpler
  • Less load on app server
  • Ignoring the migrations, this change resulting in a net reduction of 77 lines of code :)
  • Log entries not deleted when instance/server is deleted.
  • Includes a fully reversible data migration, so this can be tested, deployed, or even reverted without any loss of data.
  • The various existing logging test cases cover this code and pass with (almost) no modifications.

Cons:

  • Foreign keys not enforced at DB level.
  • (As a result:) Log entries not deleted when instance/server is deleted. (Not sure if this is a pro or a con)

Also, while I was at it I fixed an occasional pylint warning about undocumented Meta classes: We don't need docstrings on Meta, and now pylint knows that, without inline disable comments

Merge request reports