This diary is about programming, as my diaries usually are. This diary is about the code, particularly about backend codebase of the HOT Tasking Manager (TM). TL;DR of this diary is: HOT TM code is unmaintainable mess and that will not change. HOT TM developers inherited something they can do nothing about. Please, prove me wrong if you can and end up my frustration.
I will try to find out why Error when trying to split tasks happens, so deep breath and dive.
I suspect backend, so I will omit the note that it could be replicated on chrome browser only. And I will start directly at TasksActionsSplitAPI of backend/api/tasts/actions.py file, because that looks reasonable.
In the first try block I need to find out what is that SplitTaskDTO. I always forgot what “DTO” means. The import line helps and looking into backend/models/dtos/grid_dto.py says it is “DTO used to split a task”. Thanks. But the code looks like the data structure so I merely remember the purpose – just data or alike.
Only the second try left, so let’s see. The comment on the first line says it checks the project exists. I am just curious how it’s implemented, so see backend/services/project_service.py to find out exists method. Side note – I completely misunderstand classes with static methods only. Oh, exists is just Project.exists. How is that implemented? Just curious… See backend/models/postgis/project.py to see that exists is… it looks like a database query. I don’t SQLAlchemy, but note the first database request. I will number them, this one being #1.
We are back at the second try. It looks the main work is done in split_task of backend/services/grid/split_service.py, so see that (static again) method. Well, heh, not funny anymore. But I am not going to give up easily this time. So split_task now.
Get the original task by get (static) method from backend/models/postgis/task.py. In the time of writing, there is # LIKELY PROBLEM AREA comment in this method. However, it looks like simple database query again. Database query #2.
Next, we… are we really using shapely to get the geometry of the original task? It is in the database, isn’t it? So why not just request the database to give us the geometry in the format we want, like in… next line? There is #3 database query to fetch the geometry of the task? I think we already have enough original geometries of the original task, don’t we?
Next, check that the original task is big enough to be splitted. And that the task is locked. And that the user who locked the task is the user performing this request. Looks good.
We are going to _create_split_tasks. (Yes, all the methods of the SplitService are static methods.) Wait… why we need the zoom level? And, ugh, what were these x and y of the original task? I though that the task has geometry we want to split. No, we certainly create split tasks from the original task, not from its geometry. The docstring here looks outdated, at least the parameters section, but it looks it works differently for tasks that has task geometry and those that hasn’t. Or… maybe… the comment says “If the task’s geometry doesn’t correspond to an OSM tile identified by an x, y, zoom …” what task would correspond to it? Shouldn’t tasks be independent on tiles? (Do I really understand what tile is?) Let’s guess. I guess no task geometry corresponds to OSM tile, so go to _create_split_tasks_from_geometry.
Cool, it looks that _create_split_tasks_from_geometry “Splits a task into 4 smaller tasks based purely on the task’s geometry rather than an OSM tile identified by x, y, zoom”. That’s my expectation. How to do that? We have #4 to get task’s geometry as GeoJSON from the PostGIS database. But that’s not enough, we use shapely again to retrieve the task’s geometry, centroid and bounds. Good, still get that. Looks that split_geometries is a list of… yeah, it uses _as_halves and I guess that the result is four geometries representing sub-geometries of the original task’s geometry. I need a break before converting the geometries into GeoJSON features.
Good, back again. split_features is that list. For each splitted geometry we use shapely to convert it to multipolygon and then database queries #5, #6, #7 and #8 to… make it GeoJSON, I guess? But sure that _create_split_tasks_from_geometry returns the list of GeoJSON features representing splitted geometries.
Backtrack and the list of GeoJSON features representing splitted geometries is the returned by the _create_split_tasks as well.
Cool, we are back at split_task now. (It has been called from the second try block, just to remind.) We are now ready to “create new tasks from the new geojson” as the comment says. Ok. It looks that i is since now the max task’s id available in the project. I am just curious… what that get_max_task_id_for_project (static) method do? Uh, it’s #9 database query. Got it.
Now there is loop over the GeoJSON features representing splitted geometries. As the first thing we do in this loop, we… what? We check that the splitted geometry overlaps with the original geometry? Using shapely again? Really? Really.
The next block increments our i (of the max id of project’s tasks) by one and uses the incremented value to create new task from_geojson_feature. The from_geojson_feature looks innocent and I will believe that. We have create for each new task and that are database queries #10, #11, #12 and #13 – we have four splitted geometries, remember?
Oh no. Do I really need to deal with the task history now? Uh, yes, at least to check if there are some database queries. What is that task_history anyway? Oh, we copy_task_history for each splitted geometry and that copy_task_history I do not understand and I am lost and there are too many database queries. But no – no this time – don’t give up! Just pretend there is no task_history for now, and go on. I see update then, and those are database queries #14, #15, #16 and #17.
We are almost done, trying to delete the original task. If the deletion, which is #18, fails, there is some rollback and new tasks deletion and more of #…. and … raise! Generic error with no description. In the issue screenshot, it looks like there is missing information on error. I am not aware of how errors are reported, but I feel that this raise caused the error from the issue.
We are heading toward the end, doing our #18 to get project, not sure if project.tasks.count() is another database query, not sure if .filter(... and .count(... are another database queries, but save sure is #19, and we return the list of these splitted tasks as DTOs back to the second try block, returning it as the reply to the POST request for the HOT TM API. Uff.
To conclude the dive – no, for me this is unmaintainable. Just imagine two mappers splitting their task in the same time. Whoever wins max id i at #9, the other one loses. You now know where I am heading? There are #19 of the database queries in I don’t know how many files. And that’s just for split task action.