Skip to content

Allow viewing log entries in the Django Admin.#281

Merged
MarkKoz merged 11 commits into
masterfrom
simple-admin-log-entry-view
Oct 11, 2019
Merged

Allow viewing log entries in the Django Admin.#281
MarkKoz merged 11 commits into
masterfrom
simple-admin-log-entry-view

Conversation

@jchristgit
Copy link
Copy Markdown
Contributor

This PR adds log entries to the Django Admin (under the API section) to allow viewing them there.

The default overview displays log entries in chronologically descending order...

image

... and individual log entries can be inspected by clicking on them, with editing disabled to reduce clutter ...

image

... multiline tracebacks are displayed properly:

image

As I don't want to put tons of business logic in the admin this doesn't customize the HTML / CSS of it manually or anything like that.

@jchristgit jchristgit added area: backend Related to internal functionality and utilities area: frontend Related to site content and user interaction language: python Involves Python code labels Oct 11, 2019
Comment thread pydis_site/apps/api/models/log_entry.py Outdated
Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

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

I think showing both the module and the logger name is redundant in most cases though technically they don't have to be the same name. I suppose it's fine to include it. In any case, the description for the module field is incorrect since clearly that is not a fully qualified name.

@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Oct 11, 2019

Does the list support filtering?

@jchristgit
Copy link
Copy Markdown
Contributor Author

In any case, the description for the module field is incorrect since clearly that is not a fully qualified name.

Yeah, my original assumption was that it was, but for some reason getting the full module out of logging seems to be pretty hard. I can correct that in a separate PR.

@jchristgit
Copy link
Copy Markdown
Contributor Author

Does the list support filtering?

Now it does. I (well, Django) added filtering by app / timestamp / level and searching by message in f158422.

image

Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

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

Just tried this out. Filtering works well.

My concern now: can the ability to delete log entries be removed from the django admin? I feel like we would never want to do that.

@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Oct 11, 2019

Also, a nice feature to have is the ability to link to source code on our repo based on the line number and module name (and/or link in the tracebacks). Perhaps that can be saved for another PR though.

@lemonsaurus
Copy link
Copy Markdown
Contributor

I think we will probably eventually write a real frontend for this instead of using the Django admin, and that would be a good place to put nice to have features like advanced filtering, links to relevant source code, maybe even for restricting deletion.

@jchristgit
Copy link
Copy Markdown
Contributor Author

My concern now: can the ability to delete log entries be removed from the django admin? I feel like we would never want to do that.

Done.

Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

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

The merge introduced a conflicting migration that needs to be fixed:

django.core.management.base.CommandError: Conflicting migrations detected; multiple leaf nodes in the migration graph: (0044_migrate_nominations_from_infraction_to_nomination_model, 0044_add_plural_name_for_log_entry in api).

@jchristgit
Copy link
Copy Markdown
Contributor Author

fixed in 673cc3b

Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

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

Sorry. Now I noticed the "add log entry" button which I think should also be removed. I could have sworn that button wasn't there before!

@jchristgit
Copy link
Copy Markdown
Contributor Author

I don't mind it at all, no worries

Removed in 9424ae0

Comment thread docker-compose.yml Outdated
@jchristgit jchristgit force-pushed the simple-admin-log-entry-view branch from 9424ae0 to 749f157 Compare October 11, 2019 19:58
Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

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

Splendid

@MarkKoz MarkKoz merged commit 7278f75 into master Oct 11, 2019
@MarkKoz MarkKoz deleted the simple-admin-log-entry-view branch October 11, 2019 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: backend Related to internal functionality and utilities area: frontend Related to site content and user interaction language: python Involves Python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants