I think I’ll skip with PR: there is a related problem, that makes the fix (and especially its testing much more difficult): https://github.com/allegroai/clearml/issues/648#issuecomment-1102595620
I can try, but difficult to verify correctness without a publicly available test suite
The only thing I found is that I need to run flake8, but it fails even without any changes, i.e. it was not enforced before (see my msg in )
I thought that I can keep self._timeout_jobs = {} # Dict[datetime, str]
intact and fix the part where we need to extract keys instead of values.
For me - workaround is totally acceptable, thus scheduler is once again usable for me.
Did a small update: added a workaround and renamed the issue to include more client_facing conditionlimit_execution_time is present
instead of an implementation detail conditiontimeout_jobs are present
and this can break a lot of things, when somebody start the scheduler with an older version of clearml, saves the state, then upgrades and new clearml expects the state in another format
difficult to verify correctness without a publicly available test suite
What'd you mean?
I already added to the task:Workaround: Remove
limit_execution_time from
scheduler.add_task
Thanks FiercePenguin76 , I can totally understand your point on running proper tests, and reluctance to break other things.
I suggest to add a comment with the temp fix that solved the problem for you, and we will make sure the team takes it from there. wdyt?
But the second problem hints that we need to change Dict[datetime, str]
-> Dict[str, datetime]
or do some custom processing before serialization
Also added implementation thought to the issue
if a provide a PR, there I don’t see any CI processes in place that will verify the correctness of my code.