Parse more defensively #302
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This includes a failing test for a result observed with podman
The only place this is used (.WaitForHealthy()) already performs a null- safe check against the Status, comparing it to "Healthy" only. Changing the property to nullable allows more defensive parsing if it is an empty string for whatever reason, and is still interpreted as "Unhealthy" where it matters.
This adds and uses a custom converter which will parse
Container.Config.Entrypoint from a json string array I.E.
```json
{
"EntryPoint": [
"my-entrypoint"
]
}
```
Or from a single string value, I.E:
```json
{
"EntryPoint": "my-entrypoint"
}
```
This removes unused fields from the output of the `docker network ls` command, to avoid parsing errors when one or more formatted properties are unavailable on a host machine. **Warning:** This could potentially break client code, if any client actively relies on the public method: - IContainerHost.DockerHost.NetworkLs();
This removes the need to parse all output from docker top, preventing parsing errors where format of columns may differ from host to host. **Warning:** This could potentially break client code, if any client actively relies on the public method: - IContainerService.DockerHost.Top(...).Processes.Rows
Rory-Reid
force-pushed
the
parse-more-defensively
branch
from
4213c3e
to
97d7242
Compare
|
Changed my mind actually, it's not that much more effort to submit something cleaner and non-destructive. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm opening this with the full belief that these changes are far too destructive and opinionated to actually be worth merging in. This PR exists to document and demonstrate the pain points I had trying to make FluentDocker work with Podman (v 4.6.2 at the time of writing), and maybe help break the ice for #300.
It feels like Podman is too dissimilar from docker for this library to faithfully support it, or without breaking and losing the ability to directly invoke
docker network lsanddocker topvia this library (possibly other commands too).I think if there was an interest in actively supporting podman in its current form with this library, the changes I've made to parsing
network lsandtopshould instead try to parse as much detail as possible, safely. I wouldn't mind trying to tackle that myself, but I figure there's no point me spending time on it if there is no desire to support the quirks and differences of podman in FluentDocker today.Thanks for reading and thank you for this great tool.