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

Usage of go-rocket-update #42

Open
mouuff opened this issue Aug 20, 2021 · 2 comments
Open

Usage of go-rocket-update #42

mouuff opened this issue Aug 20, 2021 · 2 comments

Comments

@mouuff
Copy link

mouuff commented Aug 20, 2021

Hello,

While you are using updater.Update() you are checking if the status is UpToDate only if an error have been returned:

verbis/api/sys/update.go

Lines 62 to 74 in 1e2191c

code, err := s.client.Update()
if err != nil {
logger.Error("Executable updated failed, rolling back")
err := s.client.Rollback()
if err != nil {
logger.Panic(err)
}
switch code {
case rocket.UpToDate:
return "", &errors.Error{Code: errors.INVALID, Message: "Verbis is up to date", Operation: op, Err: err}
default:
return "", &errors.Error{Code: errors.INTERNAL, Message: "Error updating Verbis with status code: " + strconv.Itoa(int(code)), Operation: op, Err: err}
}

This will never happen because if the application is UpToDate then the updater will not return any error.
An error will only be returned when something bad happens, (for example: you are offline and cannot check for an update)

Also, you don't need to call Rollback() yourself, unless you want to rollback after successful update. (for example if you are doing an additional check by yourself to see if it is installed correctly)

The project is smart enough to rollback by itself if something bad happens during the installation :)

I hope you like the go-rocket-update project.
Don't hesitate to reach out. I will try to support everyone who is trying to adopt this library :)

Regards

@ainsleyclark
Copy link
Collaborator

Hi @mouuff

Thanks for reaching out and taking the time to review the code. I will be sure to refactor this so I check the code of UpToDate before I check for the error, great spot.

The one thing I did notice, which makes it quite difficult for testing. Is that you will notice I am setting the clients provider in the update function, see below.

verbis/api/sys/update.go

Lines 57 to 60 in 1e2191c

s.client.Provider = &provider.Github{
RepositoryURL: api.Repo,
ArchiveName: fmt.Sprintf("verbis_%s_%s_%s.zip", s.LatestVersion(), runtime.GOOS, runtime.GOARCH),
}

This is because I am unable to retrieve the latest version at runtime (as opposed to initialization). It would be great to pass in the archive name in the Update function. so I don't have to reassin the provider.

Hope that makes sense.

FYI, we're looking for contributiors for Verbis! I saw you forked it, would love to know your thoughts, or if you have time to help contribute.

@mouuff
Copy link
Author

mouuff commented Aug 21, 2021

Hello,

Thanks for the feedback, it is indeed something that could be a little easier.
Passing the ArchiveName in the Update() method is not something that I want to see because the updater needs to stay completely independent of providers.

However, I can think of a few other solutions for you:

I think the first one is pretty easy to put in place, I will try to find some time to do that.

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

No branches or pull requests

2 participants