-
Notifications
You must be signed in to change notification settings - Fork 110
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
[RSDK-7562] Add enablewebprofile to robot config #3934
Conversation
should we not just put the rest of the rdk changes in here too? so can test the flow end to end |
This is finally ready for review. App side for the config change https://github.com/viamrobotics/app/pull/4845 |
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; just a question about -webprofile
and JSON combinations.
@@ -195,7 +195,7 @@ func (s *robotServer) createWebOptions(cfg *config.Config) (weboptions.Options, | |||
if err != nil { | |||
return weboptions.Options{}, err | |||
} | |||
options.Pprof = s.args.WebProfile | |||
options.Pprof = s.args.WebProfile || cfg.EnableWebProfile |
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.
[q] So if we run the RDK with -webprofile
and put enable_webprofile: false
in the JSON config, the web profile will still be enabled?
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.
Yes that's how it currently behaves. I think that behavior makes sense as in if I add the flag explicitly I expected to just work. Happy to change it if we end up finding this to be confusing.
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.
Cool; thanks! I think what you have now makes sense 👍🏻.
Proto update here viamrobotics/api#497