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

Add BackendStatusString method to ContainerStatus #4167

Merged
merged 3 commits into from
May 7, 2024

Conversation

amogh09
Copy link
Contributor

@amogh09 amogh09 commented May 7, 2024

Summary

Currently ContainerStatus enum and backend recognized container status strings are coupled as BackendStatus method that is used to map former to latter returns a ContainerStatus and not a string. This is not ideal as the coupling restricts the evolution of internal container statuses in Agent. This PR decouples the two by introducing a new BackendStatusString method for ContainerStatus type that maps the internal container status to a suitable backend recognized status string. Existing BackendStatus method is marked as deprecated in this PR.

BackendStatusString method is slightly different from BackendStatus in that its steady state parameter is optional and defaults to ContainerRunning. That will be useful in cases where there is no special steady state for a container.

This decoupling already exists for TaskStatus type, this PR extends the same logic to ContainerStatus.

// BackendStatus maps the internal task status in the agent to that in the backend
func (ts *TaskStatus) BackendStatus() string {
switch *ts {
case TaskRunning:
fallthrough
case TaskStopped:
return ts.String()
}
return "PENDING"
}

The new method is not currently used but we plan to use it in the near future.

Testing

New tests cover the changes: yes

Description for the changelog

Enhancement: Decouple internal container status and backend recognized container status by introducing BackendStatusString() method to map internal container status to a backend recognized container status.

Does this PR include breaking model changes? If so, Have you added transformation functions?

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@amogh09 amogh09 marked this pull request as ready for review May 7, 2024 16:19
@amogh09 amogh09 requested a review from a team as a code owner May 7, 2024 16:19
@amogh09 amogh09 merged commit f14b613 into aws:dev May 7, 2024
40 checks passed
@JoseVillalta
Copy link
Contributor

late comment but this LGTM

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.

5 participants