lix-project/lix-install-action
1
0
Fork 0
Code Issues 11 Pull requests Packages Projects Releases Wiki Activity

Retry (w/ back-off) downloading the installer binary #46

Merged
colemickens merged 4 commits from retries into main 2023-10-19 13:42:24 +00:00
Conversation 3 Commits 4 Files changed 6 +4156 -7105
colemickens commented 2023-10-18 14:40:39 +00:00 (Migrated from github.com)
Copy link
Description
  • Replace node-fetch with node's builtin fetch
  • Add fetch-retry as a new dep
  • Use fetch-retry to retry and exponentially back-off on downloading the installer
  • limit to 5 retries

Tested by running the GH CI workflow in my private repo, and it should run here again.

I don't do a lot of JS/TS and that's being generous to me. Please feel free to review harshly, happy to learn anything, even if nit-y. 🙏

Additional context, per fetch-retry readme:

  • it will only retryOn network failures by default
  • (default is 3 retries, with static 1s delay, but we override, not the defaults, but the only invocation used)
Checklist
  • Tested changes against a test repository
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • (If this PR is for a release) Updated README to point to the new tag (leave unchecked if not applicable)
##### Description - Replace `node-fetch` with node's builtin fetch - Add `fetch-retry` as a new dep - Use `fetch-retry` to retry and exponentially back-off on downloading the installer - limit to 5 retries Tested by running the GH CI workflow in my private repo, and it should run here again. I don't do a lot of JS/TS and that's being generous to me. Please feel free to review harshly, happy to learn anything, even if nit-y. :pray: Additional context, per `fetch-retry` readme: * it will only `retryOn` network failures by default * (default is 3 retries, with static 1s delay, but we override, not the defaults, but the only invocation used) ##### Checklist - [x] Tested changes against a test repository - [ ] Added or updated relevant documentation (leave unchecked if not applicable) - [ ] (If this PR is for a release) Updated README to point to the new tag (leave unchecked if not applicable)
Hoverbear (Migrated from github.com) reviewed 2023-10-18 17:22:53 +00:00
src/main.ts
@ -423,8 +434,10 @@ class NixInstallerAction {
}
Hoverbear (Migrated from github.com) commented 2023-10-18 17:22:53 +00:00
Copy link

I'd really like to not use any here...

I'd really like to not use `any` here...
Hoverbear (Migrated from github.com) approved these changes 2023-10-18 17:23:49 +00:00
Hoverbear (Migrated from github.com) left a comment
Copy link

Once the any use is resolved, I think it looks fine?

Once the `any` use is resolved, I think it looks fine?
colemickens (Migrated from github.com) reviewed 2023-10-19 09:24:50 +00:00
src/main.ts
@ -423,8 +434,10 @@ class NixInstallerAction {
}
colemickens (Migrated from github.com) commented 2023-10-19 09:24:49 +00:00
Copy link

Ack, I looked at this again and slightly have a better feel for what's going on. I can convert the fileStream be Web-y and then I don't have to cast, and don't have to use any.

Ack, I looked at this again and slightly have a better feel for what's going on. I can convert the fileStream be Web-y and then I don't have to cast, and don't have to use `any`.
colemickens commented 2023-10-19 09:25:16 +00:00 (Migrated from github.com)
Copy link

New commit, more node-y naming for fetchRetry and fixed the aforementioned casting/any issue.

New commit, more node-y naming for `fetchRetry` and fixed the aforementioned casting/any issue.
colemickens commented 2023-10-19 09:45:58 +00:00 (Migrated from github.com)
Copy link

Huh. It's a bit tricky having dist/ checked in, one has to be careful to ensure that it gets rebuilt after rebasing, and it probably means that it's "wrong" for inbetween commits. Huh.

Huh. It's a bit tricky having `dist/` checked in, one has to be careful to ensure that it gets rebuilt after rebasing, and it probably means that it's "wrong" for inbetween commits. Huh.
Hoverbear commented 2023-10-19 13:41:18 +00:00 (Migrated from github.com)
Copy link

Yeah it's just 'great'.

Yeah it's just 'great'.
Hoverbear (Migrated from github.com) approved these changes 2023-10-19 13:42:17 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Hoverbear
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lix-project/lix-install-action#46
No description provided.
Delete branch "retries"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?