D22446: Adding error handling strategy control to Scheduler

[Eric Dejouhanet]

TallFurryMan added a comment.


  See my comments. This is defintely a better behavior, but I'm still not happy with JOB_ERROR :)

INLINE COMMENTS

> scheduler.cpp:3303
>                      {
> -                        appendLogText(i18n("Warning: job '%1' capture procedure failed, aborting job.", currentJob->getName()));
> +                        appendLogText(i18n("Warning: job '%1' capture procedure failed, marking aborted.", currentJob->getName()));
>                          currentJob->setState(SchedulerJob::JOB_ABORTED);

Just a warning not about translations. If you change this, translators need to change stuff, perhaps needlessly.

> wreissenberger wrote in scheduler.cpp:1431
> I would agree if JOB_ERROR is only used when an error is non recoverable. But for example what error state should be set if a slew command fails? It is definitely recoverable, since sending the command again solves the problem. The same is true if e.g. we loose network connection.
> 
> The problem is, that we do not have a JOB_FATAL state. For those, I absolutely agree, we should not try to restart. That's why I added the option to handle errors like aborts. I am quite sure that there are setups where restarting of errors is dangerous. But with my setting, its absolutely fine.

I believe the current JOB_ERROR and your example JOB_FATAL are equivalent. If the failures of a slew command leads to JOB_ERROR currently and you think it is recoverable, then we should change the state resulting from such failure to JOB_ABORTED.

> wreissenberger wrote in scheduler.cpp:1579
> This section simply extends the old predicate based to a loop so that it can take into account whether the "re-schedule errors" checkbox is selected. I needed to switch to a loop since the checkbox is not static.

OK. I'm still worried about the edge cases I listed.

> wreissenberger wrote in scheduler.cpp:3165
> Please take a look into this discussion: https://indilib.org/forum/general/5423-potential-bug-in-the-scheduler-mount-doesn-t-get-parked-when-guiding-aborted.html
> 
> It seems like the current behavior is not very intuitive regarding restarting jobs that hit limiting constraints.

Agreed, there is still work to do. From my tests, the best way to execute multi-day schedules is to automatically order targets per altitude and set targets to capture indefinitely with a high altitude restriction. This needs to be tested with your auto-restart mechanism. If of course someone is willing to spent a few clear nights to let the setup work its schedule on its own :)

> wreissenberger wrote in scheduler.cpp:3191
> I have to correct, EVALUATION does not help - see comment above. If we have the option for immediate restart selected, aborting a job with completion time exceeded leads to the situation, that it is scheduled for the following day and the next job as consequence will be shifted to the next day as well - which is not the desired behavior.

I think I understand why you changed that: it conflicts with your auto-restart mechanism. What if you set it to complete when auto-restarting, with a warning, and to aborted when not auto-restarting?

> wreissenberger wrote in scheduler.cpp:3217
> see above on line 3191.

Same as previous comment.

> TallFurryMan wrote in scheduler.cpp:3236
> I disagree: this job must be rescheduled to the next observation time, thus JOB_ABORTED.

Same as previous comment.

> TallFurryMan wrote in scheduler.cpp:4459
> As mentioned earlier, JOB_ERROR jobs should be left cancelled per definition.

Still stands.

> TallFurryMan wrote in scheduler.ui:1039
> Sorry I didn't check the UI by importing the diff. Will do.

Didn't test in the end :/

REPOSITORY
  R321 KStars

REVISION DETAIL
  https://phabricator.kde.org/D22446

To: wreissenberger, mutlaqja, TallFurryMan
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190829/882dbb24/attachment.html>