Remove internal-frontend from default services #3849
Conversation
cmd/server/main.go
Outdated
| @@ -139,6 +141,15 @@ func buildCLI() *cli.App { | |||
| return cli.Exit(fmt.Sprintf("Unable to load configuration: %v.", err), 1) | |||
| } | |||
|
|
|||
| if !c.IsSet("service") && !c.IsSet("services") { | |||
| if _, ok := cfg.Services[string(primitives.InternalFrontendService)]; !ok { | |||
| // --services is using default value (all services) but internal-frontend | |||
There was a problem hiding this comment.
Where do we set the default values for services? Why not change the default value to be explicitly the 4 roles?
There was a problem hiding this comment.
The default value is just https://github.com/temporalio/temporal/blob/master/temporal/server.go#L49-L54 , i.e. the same list that is used for validation.
So, that idea would be to add a new default value with just the four. There'd be a chance of adding a new role and forgetting to add it to the default list, but I guess if they're right next to each other that seems minimal
There was a problem hiding this comment.
(When I was working on this I had the intention of migrating everyone to use internal-frontend by default over a couple releases. It's more consistent and simpler. In that world, the original contents of this PR would be just for compatibility and could be deleted after a while, and keeping the default be "all" makes sense. But I guess we don't have consensus to go that way and it might not be worth it.)
dnr
changed the title
This reverts commit de6f2df.
This reverts commit de6f2df.
What changed?
If running without a
--serviceflag (e.g. in development or a tiny test environment), don't run internal-frontend (and don't require it in the static config).Why?
So that the new version works with old static config files.
How did you test it?
Manually
Potential risks
Only affects running without
--service, which shouldn't happen in production environments