feat: add wait flag on deploy cmd #900
Conversation
worstell
requested review from
alecthomas,
dhanji and
wesbillman
as code owners
worstell
force-pushed
the
worstell/20240207-add-wait-flag-for-deploy
branch
2 times, most recently
from
1f66d73 to
5d4dd13
Compare
| @@ -138,6 +138,7 @@ func (d *devCmd) Run(ctx context.Context, client ftlv1connect.ControllerServiceC | |||
| deploy := deployCmd{ | |||
| Replicas: 1, | |||
| ModuleDir: dir, | |||
| Wait: d.ExitAfterDeploy, | |||
There was a problem hiding this comment.
if ExitAfterDeploy is true, we'll pass Wait for all the deploys contained therein
worstell
force-pushed
the
worstell/20240207-add-wait-flag-for-deploy
branch
from
5d4dd13 to
16b1541
Compare
cmd/ftl/cmd_deploy.go
Outdated
| ticker := time.NewTicker(1 * time.Second) | ||
| defer ticker.Stop() |
There was a problem hiding this comment.
I'm not sure if it has the same effect you're looking for, but you might be able to use time.After in the select below vs. creating a NewTicker.
https://github.com/TBD54566975/ftl/blob/main/cmd/ftl/cmd_dev.go#L175
There was a problem hiding this comment.
i think we actually want ticker here instead of timer? both would probably work but from googling it seems like timer is more for performing a single event after a delay whereas ticker is intended for recurring events executed at an interval
🤷♀️
cmd/ftl/cmd_deploy.go
Outdated
| logger.Infof("Waiting for deployment %s to become ready", resp.Msg.DeploymentName) | ||
|
|
||
| wg, ctx := errgroup.WithContext(ctx) | ||
| wg.Go(d.checkReadiness(ctx, client, resp.Msg.DeploymentName)) |
There was a problem hiding this comment.
I don't think you need any concurrency here, you can just run the for loop synchronously?
cmd/ftl/cmd_deploy.go
Outdated
| } | ||
| } | ||
| if !found { | ||
| return fmt.Errorf("deployment %s not found: %v", deploymentName, status.Msg.Deployments) |
There was a problem hiding this comment.
Is the assumption here that the deployment should already exist? Is that true?
There was a problem hiding this comment.
it should be true, we're performing the wait loop synchronously after calling the controller to persist the deployment via client.CreateDeployment
worstell
force-pushed
the
worstell/20240207-add-wait-flag-for-deploy
branch
from
16b1541 to
d7ec723
Compare
fixes #885