Set query=event=workflow_dispatch&status=in_progress. Align wait_inte…

[levon-kechichian-KR]

…rval with api and docs

[levon-kechichian-KR]

@keithconvictional , need your LGTM stamp of approval XD

[neilmayhew]

I think this is a helpful change. Sometimes the triggered job gets to in_progress before the end of the first wait interval, and then the action loops forever trying to get the run id.

The job will eventually transition from queued to in_progress so with this change the action will eventually get the run id. However, it would be best to add a brief sleep in that loop, to stop the action from hammering the API while the triggered workflow is getting started.

[neilmayhew]

Also, with this change, I think the note about timing in the README can be removed or changed since it'll no longer be a problem.

[levon-kechichian-KR]

I think this is a helpful change. Sometimes the triggered job gets to in_progress before the end of the first wait interval, and then the action loops forever trying to get the run id.

The job will eventually transition from queued to in_progress so with this change the action will eventually get the run id. However, it would be best to add a brief sleep in that loop, to stop the action from hammering the API while the triggered workflow is getting started.

@neilmayhew Thank you for the feedback :)

Regarding this brief sleep, do you think it'd be better to sleep for a fixed time (say 1 second) between each call. Or should the ${wait_interval} suffice? Maybe even a separate variable altogether?

[tjg184]

Just a note here, using in_progress could result in finding the wrong workflow id too. If there's already a run in_progress before the new workflow has gotten out of queued, this will pick up the wrong workflow id.

Using queued has similar type issues since there could be something already queued before the new one is queued.

One additional check that could be helpful is to check the date/time of the run and see if its falls within a freshness threshold. In some of our examples, it was picking up an in_progress that had been running for several minutes.

Thanks for putting this together.

[xucian]

is this something you'll fix, guys?

[smfeest]

I was about to submit a PR to fix #19, but I've just noticed that the same fix is already implemented as the first commit in this PR. Since there appears to be a bit of complexity related to the other changes in this PR, is there a case for maybe merging the first commit separately to resolve issue #19?

[neilmayhew]

It seems that all the proposed schemes for getting the run ID for the triggered workflow are fragile, because there's a race condition that's very hard to overcome. For example, I've seen the triggered workflow complete before the 10s wait interval that immediately follows the dispatches API call. It's then impossible to know whether the run hasn't started yet or is already finished. IMHO, the API should return the ID of the run, and this would remove the race condition altogether. I've created a feedback discussion for this but I don't know how likely it is that the API would change any time soon.

In the meantime, I wonder if there's a more robust solution. One possibility would be to list the run IDs before and after triggering the workflow, and see which ones have been added. The created parameter could perhaps be used to limit the query to runs less than a few minutes old. This might start to be a little too complicated for a bash script, but something could be done with join, I expect.

[neilmayhew]

I implemented these changes in another PR, on top of this PR. I've tested it and it seems to work nicely. Let me know what you think.

You'll want to look at the commits individually since it includes the commits from this PR, so if you want to see what I've added look just at my two commits. I'm thinking we would merge this PR first and then (if you like it) merge mine afterwards. I would rebase mine after this one is merged.

[levon-kechichian-KR]

I implemented these changes in another PR, on top of this PR. I've tested it and it seems to work nicely. Let me know what you think.

You'll want to look at the commits individually since it includes the commits from this PR, so if you want to see what I've added look just at my two commits. I'm thinking we would merge this PR first and then (if you like it) merge mine afterwards. I would rebase mine after this one is merged.

I am totally fine with that :) Thank you for making this change!

[neilmayhew]

@keithconvictional Any chance of getting these two PRs merged soon?