DemocracyClub / WhoCanIVoteFor

🗳 The source for https://whocanivotefor.co.uk/
https://whocanivotefor.co.uk/
40 stars 31 forks source link

silence sentry DisallowedHost errors #2087

Closed chris48s closed 3 days ago

chris48s commented 4 days ago

Refs https://app.asana.com/0/1205090401342417/1208407136740662/f

Similar to https://github.com/DemocracyClub/EveryElection/pull/2311 and https://github.com/DemocracyClub/UK-Polling-Stations/pull/8152

coveralls commented 4 days ago

Coverage Status

coverage: 58.201% (-0.02%) from 58.225% when pulling e11e3b9872ff93c8e6783144d3041892f35506ee on sentry-disallowed-host into 1427a5e0f5b0c11476d9aa21a0798287704e4935 on master.

symroe commented 3 days ago

Couple of things:

  1. The file names in settings are really about environments rather than collections of settings (base.py is an exception, of course). I wonder if there's a better place for this, or a sub-folder we can make that groups settings (I can see this would be useful for, e.g pipeline settings)
  2. Using before_send is fine, but is it possible to ignore the logger with ignore_logger("django.security.DisallowedHost")?
chris48s commented 3 days ago

Yeah I tried doing it with ignore_logger which also works and is way simpler. Thanks :+1: I think given that method requires adding less code, it makes sense to just do it inline anyway, so I've got rid of the separate file. I agree that the config files themselves should correspond to environments. In WDIV, we have split the config into modules but put them in a subdirectory. So the files in https://github.com/DemocracyClub/UK-Polling-Stations/tree/master/polling_stations/settings correspond to environments but the files in https://github.com/DemocracyClub/UK-Polling-Stations/tree/master/polling_stations/settings/constants are groups of related settings. That's probably the way to do it. But also, if we don't need to define a filtering function this can probably just live in the base config. Locality of behaviour is also a thing to optimise for.

symroe commented 3 days ago

Nice one :+1: