Skip to content

Updates detection of already-running docker compose instances #20

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

Merged
merged 2 commits into from
Aug 26, 2024

Conversation

christophergunn
Copy link
Collaborator

@christophergunn christophergunn commented Aug 23, 2024

Current parsing of textural docker compose ps output assumes "--------" will delimit output. This is no longer the case. This PR amends us to use the JSON output formatter when invoking docker compose ps.

  • Adds PsWithJson method to DockerCompose.cs to use within the DockerCompose.cs CheckIfRunning method. This protects us somewhat from future changes to the text output of docker compose ps.

  • Updates tests.

I did consider adding a JSON parser library to parse the output (and would be happy to switch to that), but I did not want to bloat any consuming code bases.

Current parsing of textural docker compose ps output assumes `"--------"` will delimit output. This is no longer the case. This PR amends us to use the JSON output formatter.

* Adds PsWithJson method to DockerCompose.cs to use within DockerCompose.cs `CheckIfRunning` method. This protects us somewhat from changes to the text output of docker compose ps.

* Updates tests.

I did consider adding a JSON parser library to parse the output (and would be happy to switch to that), but I did not want to bloat any consuming code bases.
In case anyone was using it.
@christophergunn
Copy link
Collaborator Author

I’m going to merge this as the tests pass and it’s just restoring previously created functionality.

@christophergunn christophergunn merged commit 283d785 into master Aug 26, 2024
1 check passed
@christophergunn christophergunn deleted the fix-docker-teardown-on-start branch August 26, 2024 07:52
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.

1 participant