I the number should be fine, but we should atleast have some benchmarks on the number, if it affects the ES cluster in anyway.

Sounds good. I'll see if QA is able to do some load testing, otherwise I'll look into doing this.

No need to change this here, but just useful to know about. Since Python 3.6, you can use underscores to separate thousands in decimals (and words in hexadecimals and nibbles in binaries) to make numbers more readable. e.g.

MAX_GEOSPATIAL_INDEX_DOC_LIMIT = 1_000_000
addr = 0xCAFE_F00D
flags = 0b_0011_1111_0100_1110

@kaapstorm Initially this is how I had it but then ultimately decided against it since I didn't see this formatting being used anywhere else within the settings.py file. I think this is fine to keep as is, but good to keep in mind for next time!

🥇

Nit: It feels awkward for the tasks module to be importing from a management command, instead of the other way round (or both importing from somewhere else, but I'd prefer not to throw everything in the utils module, like that second drawer in the kitchen that somehow ends up not only with tongs and skewers, but also clothes pegs, and screws, and one rusty paper clip).

Perhaps a good alternative would be to move the es_case_query function off to the es.py file as I feel it would fit well there. This can then be renamed to something like es_case_query_for_missing_geopoint_val or something to that effect.

Alternatively, I can also simply create a file in a new "utils" directory to contain just the above helper function. I do feel the first option makes sense enough and is a bit more straightforward, but happy to go in this direction as well if you think it sounds more suitable @kaapstorm.

The es module sounds ideal for es_case_query! Yeah, I agree, the function is worth renaming too, and you could drop the "es_" prefix because we would know from its module that it would be an Elasticsearch query.

Addressed in a161b10.

If you're importing a protected function somewhere other than a test, then it's possibly no longer being treated as protected and you should drop the leading underscore. -- This is a guideline, not a rule, so I'm not requesting a change, and I'll leave it to your discretion ... but interesting that the linter didn't flag this.

Good point. I have always been a little unsure as to how strict we should be when it comes to marking functions in Python as private. Is having simply a single external reference enough justification, or do we need to start referencing it a few times before it becomes obvious that this is clearly a public function (I have seen plenty of examples in HQ that use both examples)? I suppose this is the potential downside of having it more as a guideline than a rule, it can be difficult to gauge where the line lies at times.

Thinking this through however, the former does make more sense since even a single external reference clearly means its not private/self-contained anymore. Given this, I agree it would make sense to drop the _ here.

nit: how do we ensure this is followed up on?

The domain will receive an error alert if the Celery task fails, asking them to reach out to support. Sending this off to Sentry will allow us to have a bit more insight other than a generic CELERY_ERROR message to understand what could have gone wrong.

Cool, that works 👍