Fixes viz issues triggered by the task_id change #1542
Conversation
|
lgtm 🎉 |
Server-side filtering of tasks broke when task ids changed from the string representation of a Task object to a truncated, cleaned up combination of parameter values and a partial md5 hash. To fix this, we reconstruct a pretty id more similar to the old value that was matched against for server-side task filtering.
daveFNbuck
force-pushed
the
viz_display_names
branch
from
d6d4d24
to
897c9d9
Compare
|
Accidentally used something from #1538, re-uploaded with a small tweak to how the unit tests for server-side filtering generate task ids so the tests can pass. |
|
I've noticed that the new task ids make it unsafe to upgrade workers while other workers for the same tasks are still alive. The new workers will schedule jobs with different task ids and the scheduler will treat them as different tasks. |
|
that's weird – what's causing that? |
|
The task id used to be Task(param=val) but now it's like Task_val__a123aef5. So if you have two workers add the same task with these two different ids, the scheduler keys storage on these different ids and doesn't know that the tasks are the same. |
|
the latter a123aef5 part should be a hash of the parameters. that's not good. i wonder what's not working |
|
Nothing isn't working, you just can't have workers with versions of luigi before and after the task id change doing the same jobs because they'll have different task ids for the same tasks. It's just a concern while upgrading. Once everything is upgraded, there is no problem. |
|
oh, got it. yes that's a good catch. i guess now that the new task id's have been merged for a few months it's probably less urgent to fix. we should remember it next time we upgrade the task id's though |
|
Not yet released to pypi. |
|
i didn't realize that. is there an easy way to fix it? i'm not convinced this is a serious issue |
|
I think it's just something we need to try to warn users about when we release to pypi. Maybe mention it in the google group or something when announcing the new version. |
|
Sounds good |
Fixes viz issues triggered by the task_id change
With the task_id change, a couple issues cropped up in the visualiser. Most annoyingly, server-side filtering broke because the task_id no longer contains the same data shown in the data table. This is fixed by reconstructing a task_id similar to the old one for server-side filtering.
Less annoyingly, task display names in the visualiser changed from TaskClass(param=val) to TaskClass({"param": "val"}). This is fixed by passing the display name computed for filtering in the server to the visualiser for display.
Finally, there were a few places in the visualiser where the new task id was displayed: the first tasks in the workers tab, the graph name in the graph tab, and error popups in the graph tab. All three of these are fixed to show a nice display name that is more helpful to users.
We should probably also show a nicer display name in worker error messages, but this PR is focusing on the visualiser.