feat: Skip syncing on start if flag set #485
Conversation
cmd/dev_server/start_server.go
Outdated
| @@ -55,7 +58,7 @@ func startServer(client dev_server.Client) func(*cobra.Command, []string) error | |||
| if viper.IsSet(cliflags.ProjectFlag) && viper.IsSet(SourceEnvironmentFlag) { | |||
|
|
|||
| initialSetting = model.InitialProjectSettings{ | |||
| Enabled: true, | |||
| Enabled: viper.GetBool(cliflags.SyncOnStartFlag), | |||
There was a problem hiding this comment.
I think we are looking for behavior that is "only sync 1 time".
that way a customer could do:
ldcli dev-server start --project default --source staging --project default --source staging --sync-once true
That way the consumer does not need to vary their input for starting the server (without resyncing if the project/env exists)
There was a problem hiding this comment.
Yes. However, if we can make the cli flag be --sync-once instead of --sync-once true, I will be happier 😉
There was a problem hiding this comment.
@mike-zorn I looked into it, it looks like the default behavior is what you want. However to achieve that Cobra has some quirks.
tl;dr to set the flag to false you either have to not include the flag at all or use an = e.g. --sync-once=false which makes bool flags unique among other Cobra flags. What I'm seeing in the code itself backs that up.
internal/dev_server/model/sync.go
Outdated
| if !errors.Is(createError, ErrAlreadyExists) { | ||
| return createError | ||
| } |
There was a problem hiding this comment.
Moved the logic around to clean up the logic and hopefully make it easier to grok
internal/dev_server/model/sync.go
Outdated
| if settings.SyncOnce { | ||
| log.Printf("Project [%s] exists, but --sync-once flag is set, skipping refresh", settings.ProjectKey) | ||
| } else { | ||
| log.Printf("Project [%s] exists, refreshing data", settings.ProjectKey) | ||
| var updateErr error | ||
| project, updateErr = UpdateProject(ctx, settings.ProjectKey, settings.Context, &settings.EnvKey) | ||
| _, updateErr = UpdateProject(ctx, settings.ProjectKey, settings.Context, &settings.EnvKey) |
There was a problem hiding this comment.
Going to add a unit test for this
| @@ -10,6 +10,7 @@ Supported settings: | |||
| - `output`: Command response output format in either JSON or plain text | |||
| - `port`: Port for the dev server to run on | |||
| - `project`: Default project key | |||
| - `sync-once`: Only sync new projects. Existing projects will not be resynced on startup | |||
There was a problem hiding this comment.
| - `sync-once`: Only sync new projects. Existing projects will not be resynced on startup | |
| - `sync-once`: Only sync new projects. Existing projects will neither be resynced nor have overrides specified by CLI flags applied. |
Requirements
Related issues
https://launchdarkly.atlassian.net/browse/FUN-447
Describe the solution you've provided
Added a
should-syncflag to the server startup command that setsmodel.InitialProjectSettings.Enablewhich controls whether this function no-ops or not.Using the no-op made sense for the persistent volumes use case, since all the project and env data will already exist locally, but open to being more surgical if so desired.
Describe alternatives you've considered
We could skip the
UpdateProjectcall specifically in theCreateOrSyncProjectfunction, which does the sync bit. Then we'd have to decide if we still want to do the overrides.Additional context
Add any other context about the pull request here.