skip _sis_runnable check when already finished

[albertz]

Via: #167 (comment)

See the corresponding discussion there for why I did this.

However, I'm not really sure that this is correct or wanted. I think this mostly makes a potential difference for jobs which have a custom update function and which might dynamically add some further dependencies (and maybe also dynamically register further outputs). Now, when such a job is finished, it might never call the update, and then you will never get the full graph again. This is what I also want here, to have it faster, but there might be other cases where you would not want this?

I'm also not totally sure about the return_when_finished default. But if this default would be None, then there are likely some places where this function is called with this default, and then it anyway would do the update and expand the graph, so that would not really work.

I'm mostly leaving this PR open here for discussion.

[michelwi]

potential difference for jobs which have a custom update function and which might dynamically add some further dependencies

does the MT pipeline contain such things?

[critias]

An side effect of _sis_runnable is that it updates the whole dependency tree. Looking at the code I didn't see a case were this change should be a problem, but this change should be tested a while in practice before submitting it.

[albertz]

One side effect I realized later: When some finished job never calls its update method, there are potentially some jobs missing in the graph. E.g. when you want to perform cleanup, or when you changed your aliases and you want that it updates them, then it would miss those. This might not be so critical, but this could be unexpected. But this is maybe then also a user choice, like faster startup vs having the complete graph?

[Zettelkasten]

does the MT pipeline contain such things?

As far as I can tell, no, we don't override update() anywhere.