Skip to content

Add context where appropriate#2

Merged
stapelberg merged 1 commit into
gokrazy:mainfrom
damdo:add-context-to-target-reboot-rebootwithkexec
Jun 1, 2025
Merged

Add context where appropriate#2
stapelberg merged 1 commit into
gokrazy:mainfrom
damdo:add-context-to-target-reboot-rebootwithkexec

Conversation

@damdo
Copy link
Copy Markdown
Contributor

@damdo damdo commented May 29, 2025

This is to allow cancelling the calls from the caller side.
e.g. in the Reboot() case, where once the caller process (normally a gokrazy managed process) receives the SIGTERM signal which Reboot() itself happens to trigger via its calling of the killSupervisedServicesAndUmountPerm() function, to avoid a blocking behaviour until SIGKILL kicks in.

Context for this change here: gokrazy/selfupdate#4 (comment)

@damdo damdo changed the title Add context to target.Reboot() and target.RebootWithKexec Add context to target.Reboot(), target.RebootWithKexec() May 29, 2025
@damdo
Copy link
Copy Markdown
Contributor Author

damdo commented May 29, 2025

cc. @stapelberg

@damdo
Copy link
Copy Markdown
Contributor Author

damdo commented May 29, 2025

Once we update the users of this pkg we will need to update the func calls to pass in the context.

@stapelberg
Copy link
Copy Markdown
Contributor

This will require code changes from users (as you identified), e.g. https://github.com/ssttevee/jitaku-dns/blob/trunk/internal/update/update.go (cc @ssttevee as a heads-up)

Given that users need to adjust their code, I think we should introduce a context argument to all methods that send HTTP requests, not just the ones you happen to need right now :) Can you update the PR accordingly please?

I’ll take care of the io.ReadAll change separately so that you can keep your PR clean.

to be able to cancel it from the caller side.
Ref: gokrazy/selfupdate#4 (comment)
@damdo damdo force-pushed the add-context-to-target-reboot-rebootwithkexec branch from 12bc3f7 to aabeb55 Compare May 31, 2025 10:00
@damdo damdo changed the title Add context to target.Reboot(), target.RebootWithKexec() Add context where appropriate May 31, 2025
@damdo
Copy link
Copy Markdown
Contributor Author

damdo commented May 31, 2025

@stapelberg good suggestions thanks! Done :)

@stapelberg stapelberg merged commit 9164932 into gokrazy:main Jun 1, 2025
1 check passed
@stapelberg
Copy link
Copy Markdown
Contributor

Thanks!

@stapelberg
Copy link
Copy Markdown
Contributor

I also took care of the gokrazy/tools update: gokrazy/tools@ab76ef5

@stapelberg
Copy link
Copy Markdown
Contributor

Also gokrazy/bakery and gokrazy/selfupdate: gokrazy/bakery@a12069c and gokrazy/selfupdate@6a7c12e

And also sent PRs for downstream users:
evcc-io/evcc#21564 and ssttevee/jitaku-dns#1

@damdo
Copy link
Copy Markdown
Contributor Author

damdo commented Jun 1, 2025

Thanks, and thank you for updating the users of this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants