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

fix: wrap errors in ReplaceDeployment, don’t hide errconflicts #2108

Merged
merged 3 commits into from
Jul 18, 2024

Conversation

matt2e
Copy link
Collaborator

@matt2e matt2e commented Jul 18, 2024

also logs now include old and new deployment keys

@matt2e matt2e requested review from gak and wesbillman July 18, 2024 00:48
@matt2e matt2e requested a review from alecthomas as a code owner July 18, 2024 00:48
@matt2e matt2e requested review from a team and deniseli and removed request for a team July 18, 2024 00:48
@@ -724,24 +724,28 @@ func (d *DAL) SetDeploymentReplicas(ctx context.Context, key model.DeploymentKey
return nil
}

var ErrReplaceDeploymentAlreadyActive = errors.New("deployment already active")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know if this is how we do it anywhere else, defining an error for a specific func. Is this kosher?

@ftl-robot ftl-robot mentioned this pull request Jul 18, 2024
@matt2e matt2e force-pushed the matt2e/wrap-replace-deploy-errs branch from e74c23b to 9cabe4b Compare July 18, 2024 01:16
@matt2e matt2e merged commit b5bab9a into main Jul 18, 2024
59 checks passed
@matt2e matt2e deleted the matt2e/wrap-replace-deploy-errs branch July 18, 2024 01:22
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