Query for previous runs at every iteration of the Waiter #4
Conversation
…ting tests and add a new one for validating waiting for multiple previous runs.
|
Ping @softprops. |
|
This is on my list. Full transparency I'm a new dad taking care of a new born. My turn around time is a little slower as a result |
Congratulations 🎉👏🎉
No rush. Just wanted to put it on your radar. Happy to re-engage whenever you find some time to review this. |
| ) { | ||
| this.info(`🤙Exceeded wait seconds. Continuing...`); | ||
| return secondsSoFar || 0; | ||
| } | ||
| const run = await this.getRun(); | ||
|
|
There was a problem hiding this comment.
An aha thought accurred to me when trying to understand the implications of changing the sort order above.
Previously we looked for all in progress runs filtering for the previous run then the waiter held a ref it it's I'd to poll for it's completed status.
Now that we are fetching all in progress runs inside the waiter, we technically don't need to check for completed status of any run since we only get back in progress runs.
I think that we can get away with just checking if there are any runs in this in progess list that have a run id less than the current run id.
Make sense? I'm happy to merge and follow up unless you want to grab that simplification here.
There was a problem hiding this comment.
Yeah you are right. Good catch. I have updated the PR to reflect this change.
src/wait.ts
Outdated
| this.info = info; | ||
| } | ||
|
|
||
| wait = async (secondsSoFar?: number) => { | ||
| const runs = await this.githubClient.runs( |
There was a problem hiding this comment.
Can we move this API call below the check for exceeded seconds. If that condition is met this will have been a wasted api call
…ly if wait has exceed prescribed wait time.
|
@softprops I have addressed both of your comments. I also hope you don't mind that I updated the GHA workflow file to run the |
|
Thanks for all of this. I'll push a tag tomorrow |
|
updated the v1 tag. this should be "live" now. thanks for your help on this @praneetloke |
Fixes #3.
I believe this should fix the issue where the Action was querying for previous runs. I have also updated the sort order to be descending, so that each run awaits its immediate previous run.
Also, updated existing tests and added a new one for validating waiting for multiple previous runs.
Please feel free to use any part of this PR to your liking if you feel you would like to do something different to fix what we discussed in #3.