-
Notifications
You must be signed in to change notification settings - Fork 1
Truncate logs to 10k #999
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
base: main
Are you sure you want to change the base?
Truncate logs to 10k #999
Conversation
|
What does "too large" mean in this context, and is it worth explaining that to the user (to fend off extraneous support requests)? |
Do you have some suggested language? |
78d7bbe to
2046599
Compare
No, because I don't know what "too large" means in this context 😄 |
evansd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Very much appreciating the newly hairless yaks we now have wondering around.
Also includes a simplyfying refactor of the template caching. Currently, the templates for content rendering are loaded at import time. This is before settings.py has properly been set up, which means that the template tags available to these import-time loaded templates does not include slippers tags. Until now that hasn't mattered, but we want to be able to use them in content rendering templates. The solution is to always dynamically fetch the template on use. This is fast in prod, as django caches template loads. It's slower when DEBUG=True, but we're already working with that for our other templates. This insight into how django caches template prompted a simplifying change in the design of template caching. Previously, we had been manually caching it, which required some complex logic to manage in dev, where we want to invalidate the caching if the file is changed underneath us. Instead, now we are using django's default template loading (cache in prod, always reload in dev), we can use it to simplify things greatly. We now calculate the hash of the template contents once and store it on the template object as a custom `airlock_` prefixed attribute. In prod, the contents of the template will never change anyway, and django will always return the same cached Template object. In dev, the template is always reloaded, and new Template instance created. So we will need to recalculate the hash every time. Whilst a little inefficient, it ensures the hash will always be correct, picking up changes to the content automatically. Also switched to md5, as is much faster than SHA1, and suitable for our purposes.
Configurable in settings. For better UX, we take care to strip any partial lines that have been mangled by the truncation. We also display a message that the file has been truncted due to length. We use a slippers component to do this, which is only possible because fo the previous rework of RendererTemplate to lazy load the template. Note: this works, but the styles for the component are not loaded, as currently the content rendering templates are very bare bones. Will fix this in a subsequent commit
This ensures we have all the styles/scripts we need, and proper html page, so we can use our components if needed, but is free from the general styling (e.g. header) that the main base.html provides, so suitable for including in an iframe. The csv template already included this, but this change factors that out into a shared base template used by both the free text and csv templates. Along the way there's few small QoL changes 1) use the filename as the page title 2) don't add additional newlines to preformatted text. This was unoticed before, but now that we have a proper html template, with indentation, it was obviously incorrectly adding the indentation inside the <pre> tag.
2046599 to
af919b5
Compare

Truncate logs to the last 10k, taking care to not show partial lines.
The truncation was actually fairly simple, but our file content rendering
framework required some reworking to be able to use slippers components to be
able to tell the user that the log had been truncated.
The end result is a solid improvement though, in particular the template
caching busting is much simpler and more robust in dev, and our content
templates now have proper base template and styles.
Fixes #990