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

Add option to disable request logs #887

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

dmarcoux
Copy link
Contributor

Closes #785

@dmarcoux
Copy link
Contributor Author

I tried to test this locally by adding env = LD_DISABLE_LOGGING=true to uwsgi.ini and spinning up the server with uwsgi --http :8000 --module siteroot.wsgi. The logs were still being shown. I am not super familiar with uWSGI, maybe someone could guide me here?

@sissbruecker
Copy link
Owner

It's an environment variable, so you either set it using export LD_DISABLE_LOGGING=1 when testing locally, or adding -e LD_DISABLE_LOGGING=1 when starting the Docker container.

The change seems to work. Naming isn't great, as it doesn't really disable logging:

  • It only disables request logs
  • It will still log failed requests
  • It will still log stuff from linkding itself

Apart from that the option should be documented in the options docs.

@dmarcoux
Copy link
Contributor Author

dmarcoux commented Oct 16, 2024

My first attempt to test this was with LD_DISABLE_LOGGING=1 python3 manage.py runserver, but I could still see the request logs (for example from the /health endpoint). This is from linkding, so it stays, right?

Anyway, it's true that the naming isn't great. Maybe LD_QUIETER_LOGGING? Naming things is hard 😄

@sissbruecker
Copy link
Owner

sissbruecker commented Oct 16, 2024

My first attempt to test this was with LD_DISABLE_LOGGING=1 python3 manage.py runserver

That command uses the Django dev server, and not uwsgi. I guess LD_DISABLE_LOGGING=1 uwsgi --http :8000 --module siteroot.wsgi should do it then.

Regarding naming, LD_DISABLE_REQUEST_LOGS would be more appropriate IMO, if not 100% correct. But could be fine if the docs mention that failed requests are still logged.

@dmarcoux dmarcoux force-pushed the disable-logging-ENV branch from 8b19d89 to 21c130e Compare October 16, 2024 20:05
@dmarcoux dmarcoux changed the title Add disable-logging environment variable Add option to disable request logs Oct 16, 2024
@dmarcoux dmarcoux force-pushed the disable-logging-ENV branch from 21c130e to 0d60780 Compare October 16, 2024 21:00
@sissbruecker sissbruecker merged commit 6548e16 into sissbruecker:master Oct 17, 2024
@dmarcoux dmarcoux deleted the disable-logging-ENV branch October 18, 2024 18:47
@aerz
Copy link

aerz commented Nov 20, 2024

Hi, the option values don't use the capitalized format. Shouldn't they be True or False like the other options?

Thanks @dmarcoux for this feature!

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.

Add disable-logging environment variable
3 participants