Current state in monitorJob

[jneijt]

Currently one has to wait the set interval until the first logs are available. Especially when the interval is set to a higher timeout, the user has no feedback at the beginning. This means that if we were to show the logs in a UI somewhere, we have to kind of copy the functionality of the monitorFn in monitorJob and run it ourselves before starting monitorJob to be able to show something until the first timeout.
This would work of course but I personally think it would be a nice addition if the monitorJob function would take care of this already.

What do you think?

[m-mohr]

I had that implemented first (by having let lastStatus = null;), but then thought it is likely not necessary as I thought it's not very useful to get a callback if there's no logs available. If there's logs available then it runs anyway at the beginning. If you need a first call to the callback at the beginning always, why not just call your callback once yourself? That way you have full control over the flow. So I'm not 100% sure what the benefit is here? Is it to get the Job infos from describeJob? Then that would make sense.

[m-mohr]

Ah, wait. Now I understand... the interval doesn't execute the callback at the start. It always waits the interval once... Okay, makes much sense! Should it always run, also if there are no logs?

[m-mohr]

Actually I just found that I mistakenly used setTimeout instead of setInterval. See #28

[jneijt]

Exactly, the interval doesn't execute the callback in the beginning, only after the timeout has passed once. The way I was using it now was:

1. Fetch the job infos using describeJob to prepare a simple overview over the job in general
2. Start monitorJob and in my callback add the logs and (if available) results to that page

In this case, the page would not show any logs until the interval has passed once. Of course I could go and fetch the logs myself once but that somehow felt a bit like a duplicate.

I don't think it needs to run if there are no logs though, it doesn't really fetch any new information then, right? But do you know if there are any logs before running the monitorFn at least once (which already does what we want)?

[m-mohr]

Here would be my approach: #28
Would that work for you and help you out?

The question is whether it needs to run once just to call describeJob... I guess not? At least createJob also calls it.

But do you know if there are any logs before running the monitorFn at least once (which already does what we want)?

Could be.

[jneijt]

Aaah, now I get the point with the lastStatus. I completely missed that, makes sense now. I'll quickly test your proposed solution, looks like it should work.

[jneijt]

Yep, looks good from my side and the quick test did exactly what I was looking for.

Is this now also according to what you intended with lastStatus?

I don't think it has to run the callback just for the sake of calling describeJob, I think you probably already have that info before calling monitorJob and otherwise it's easy to add just one call to that manually in your own code. But for the logs it's nice to have.

[m-mohr]

Hmm... I'm back and forth with it. For some use cases it's useful to call describeJob, for some not so much. So I guess I just leave it as it is in the PR, which means the callback is always called once at the beginning with the updated job details.

[jneijt]

I don't think it hurts to run the callback with an updated job and no logs in the beginning, even if this is the exact same job which the user (maybe) already has. So the PR should be fine I think.
On the other hand, just calling it whenever the status has changed or if there are new logs seems reasonable to me as well. However, I'm not sure I understand all the use cases here yet.

Which use case would benefit from getting the updated job in any case right away? When you create the Job object directly with new Job(connection, id) instead of retrieving it with something like connection.getJob(id)?

[m-mohr]

The use case I'm thinking about is the Web Editor, where the user starts monitorJob maybe just a minute after creating the job. For other use cases where you just start monitoring directly after creating the job, it's likely not getting you a different result.

[m-mohr]

Okay, I just implemented the approach that describeJob is only called at the beginning if the data has not been refreshed recently (in the last second) and then it calls the callback if the status changed or logs are available (i.e. older than the time specified in the interval). I think that caters for both use cases best.

[m-mohr]

Has been merged. I'll make a rc3 release later today.

[m-mohr]

The new version has been pushed to npm.

[jneijt]

Just updated the app to the newest version locally and this works perfectly, thanks a lot! 👍