Skip to content
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

update for handle failed deployment case #501

Merged
merged 13 commits into from
Jul 5, 2024

Conversation

nullfunc
Copy link
Contributor

@nullfunc nullfunc commented Jun 25, 2024

Handle FAILED deploy status

#491

DO NOT MERGE until https://github.com/DefangLabs/defang-mvp/pull/762 is approved

update message to show error or success

update error text

update text
@nullfunc nullfunc requested a review from lionello June 25, 2024 23:11
src/cmd/cli/command/commands.go Outdated Show resolved Hide resolved
src/cmd/cli/command/commands.go Show resolved Hide resolved
@lionello
Copy link
Member

I want us to fix up the existing serviceInfo.Status field to match the Status returned by Subscribe. I think both of these should be the same and should be an enum. The serviceInfo.Status field will also contain a free form string, which we'd have to create a new field for (or rather, put the string in the existing "Status" field but add the new enum field.)

@nullfunc nullfunc force-pushed the eric-handle-subscribe-deploy-failed-case branch from 968bfd5 to 4701a2b Compare June 27, 2024 16:50
@nullfunc nullfunc requested a review from lionello June 27, 2024 18:23
src/pkg/cli/compose/convert.go Outdated Show resolved Hide resolved
src/pkg/cli/tail.go Show resolved Hide resolved
src/protos/io/defang/v1/fabric.proto Outdated Show resolved Hide resolved
UNKNOWN = 0;

// Build states (initial state for build)
BUILD_PENDING = 1; // BUILD_QUEUED -> BUILD_ACTIVATING
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why these aren't simply 1:1 the existing ones? Seems not too much gain for lot of confusion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to map them to something platform agnostic but probably a little early to be doing that.

src/protos/io/defang/v1/fabric.proto Outdated Show resolved Hide resolved
CREATED = 3; // BUILD_DEACTIVATING (done = true)

// Update states (initial state for existing image)
UPDATE_PENDING = 4; // UPDATE_QUEUE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, why the rename?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will change back.

@nullfunc nullfunc requested a review from lionello July 3, 2024 17:56
src/cmd/cli/command/servicemonitor.go Show resolved Hide resolved
src/pkg/cli/client/byoc/aws/byoc.go Outdated Show resolved Hide resolved
src/cmd/cli/command/commands.go Show resolved Hide resolved
if err := waitServiceState(ctx, defangv1.ServiceState_SERVICE_COMPLETED, serviceInfos); err != nil && !errors.Is(err, context.Canceled) {
if errors.Is(err, ErrDeploymentFailed) {
term.Warn("Deployment FAILED. Service(s) not running.")
cancelTail()
Copy link
Member

@lionello lionello Jul 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lionello lionello merged commit 8fe89b4 into main Jul 5, 2024
5 checks passed
@lionello lionello deleted the eric-handle-subscribe-deploy-failed-case branch July 5, 2024 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants