Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updateProcess: fix incorrect arguments, swap them back in correct order #5

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

mpl
Copy link
Contributor

@mpl mpl commented Jan 14, 2025

updateProcess was called with the correct arguments only in the "run once" case, but not in the periodic run case.

updateProcess was called with the correct arguments only in the "run once" case, but not in the periodic run case.
Copy link
Collaborator

@damdo damdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I have fixed this issue locally in the PR I'm working on, it still hasn't made its way here though. Thank you for fixing it!

@damdo
Copy link
Collaborator

damdo commented Jan 14, 2025

cc. @stapelberg for the merge

@mpl
Copy link
Contributor Author

mpl commented Jan 14, 2025

hah. I was wondering if you all were just using selfupdate in the run once case. :)

Since we're here, I'd like to run something by you:
I find it a bit inconvenient that there is no mode where you are in the periodic case, but where there is also an immediate run nonetheless.
So I was thinking the code could become something like:

	log.Print("skipping waiting, performing an immediate updateProcess")
	if err := updateProcess(ctx, gusCli, machineID, o.gusServer, sbomHash, o.destinationDir, httpPassword, httpPort); err != nil {
		// If the updateProcess fails we exit with an error
		// so that gokrazy supervisor will restart the process.
		return fmt.Errorf("error performing updateProcess: %v", err)
	}

	if o.skipWaiting {
		// If the updateProcess doesn't error
		// we happily return to terminate the process.
		return nil
	}

Would you be against something like that? (Do you want me to open an issue about it to discuss it?)

@damdo
Copy link
Collaborator

damdo commented Jan 14, 2025

hah. I was wondering if you all were just using selfupdate in the run once case. :)

I was using it in the run once yes, so I only caught this last weekend, when I changed modes in development.

Let's open a separate conversation for the other one :)
I'll create an issue now.

@damdo
Copy link
Collaborator

damdo commented Jan 14, 2025

Created #6 for that @mpl @stapelberg

@stapelberg stapelberg merged commit df3187b into gokrazy:main Jan 14, 2025
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.

3 participants