Fix issue with tablet_template_url in HealthCheck#3959
Fix issue with tablet_template_url in HealthCheck#3959michael-berlin merged 2 commits intovitessio:masterfrom
Conversation
* Without this change, loadTabletUrl is only called during init method and it won't have the flag parsed yet. Signed-off-by: Rafael Chacon <rafael@slack-corp.com>
michael-berlin
left a comment
michael-berlin
left a comment
There was a problem hiding this comment.
Makes sense :)
LGTM after my comment is addressed.
go/vt/discovery/healthcheck.go
Outdated
|
|
||
| func init() { | ||
| loadTabletURLTemplate() | ||
| LoadHealthCheckTabletURLTemplate() |
There was a problem hiding this comment.
This should be removed then?
Instead, add a comment to the function "LoadHealthCheckTabletURLTemplate" that it should only be called after the flags are parsed?
There was a problem hiding this comment.
I left it intentionally because there was one template test that was failing without it. Fixing the test is straightforward, but I was worried that there might be a case out in the while where the template is used before LoadHealthCheckTabletURLTemplate is called. To be safe, I decided to leave the init.
Are you confident that is safe to remove?
There was a problem hiding this comment.
Okay, scratch what I said earlier. I understand the problem now.
I suggest the following:
- Rename the method to
ParseTabletURLTemplateFromFlag. - Add here a comment: "Flags are not parsed at this point and the default value of the flag (just the hostname) will be used."
- And a comment in the other file: "Flags are parsed now. Parse the template using the actual flag value and overwrite the current template."
What do you think?
There was a problem hiding this comment.
I think that makes. It will be clearer this way.
Signed-off-by: Rafael Chacon <rafael@slack-corp.com>
2 participants
Description
init()is called.Tests