-
Notifications
You must be signed in to change notification settings - Fork 113
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
Remove HTTP versions of RestartChecker, Config Reader, Log Writer #1959
Conversation
127fa91
to
885fd07
Compare
have you tested this manually? |
Yeah just tested locally with a new device. No issues with restart, logs, or getting config. Still one test failing that im looking into. |
885fd07
to
f581212
Compare
if err != nil { | ||
var urlErr *url.Error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gRPC will never return a url.Error so the check (!errors.As(err, &urlErr) || urlErr.Temporary())
is meaningless. To avoid the test now failing i disabled cert reading on the calls from the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The http -> grpc migration is long done now and those code paths are no longer needed.
f581212
to
d57dac5
Compare
|
Warning changing any of the following functions will break code samples on app if an api for these function changes please contact the fleet team
|
…amrobotics#1959) The http -> grpc migration is long done now and those code paths are no longer needed.
The http -> grpc migration is long done now and those code paths are no longer needed.