Run update check when any client command is executed #2909
Conversation
|
|
Users don't run In the next PR, we will introduce a mechanism to only do this once per day (or whatever period) to print the message less often. |
There was a problem hiding this comment.
Looks good, but when I tried this manually with devstack it tells me there's an update for every command. And if I were being picky, the text output at the end of the main command's output is too squashed up, could do with some vertical whitespace. Have approved but would be nice to fix the devstack issue
| @@ -213,6 +217,9 @@ func (fsr *FsRepo) Init() error { | |||
| if err != nil { | |||
| return err | |||
| } | |||
|
|
|||
| // TODO this should be a part of the config. | |||
| telemetry.SetupFromEnvs() | |||
There was a problem hiding this comment.
Does this need to be in Open and Init?
There was a problem hiding this comment.
I think our conclusion was maybe? And we're not confident enough to consolidate this.
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
| const endpoint = "http://update.bacalhau.org/version" |
There was a problem hiding this comment.
Can we have a comment/todo to move this to configuration somewhere, otherwise we're at risk of doing a release just to satisfy a move to https or a diff domain?
There was a problem hiding this comment.
The reasons for this are:
- We do actually want everyone to always use this domain, and it shouldn't be configurable
- It's actually easier to migrate a hard-coded constant than a configuration value right now because we don't have any config migraiton capability
If the concern is about testing, there's a few ways we can do that even with a hard-coded value (e.g. a mock http client).
This commit automatically kicks off a background update check whenever any client command is executed and prints the result at the end of the command. This means that update notifications will be reported as part of normal client usage rather than just when requested using the version command.
simonwo
force-pushed
the
2893-update-checks
branch
from
5cc83cb
to
d2ad7db
Compare
|
Discussed with Ross and comments are above. Broadly we're happy to merge this as long as we get the "once a day" PR out next. So I'll pick that up ASAP. |
This commit automatically kicks off a background update check whenever any client command is executed and prints the result at the end of the command. This means that update notifications will be reported as part of normal client usage rather than just when requested using the version command.
Resolves #2893.