gabrieldemarmiesse / python-on-whales

An awesome Python wrapper for an awesome Docker CLI!
MIT License
546 stars 101 forks source link

There was an error parsing the json response from the Docker daemon. #190

Closed WolfgangFahl closed 3 years ago

WolfgangFahl commented 3 years ago

Traceback (most recent call last): File "/home/wf/.local/lib/python3.8/site-packages/python_on_whales/client_config.py", line 214, in _fetch_and_parse_inspect_result return self._parse_json_object(json_object) File "/home/wf/.local/lib/python3.8/site-packages/python_on_whales/components/container/cli_wrapper.py", line 58, in _parse_json_object return ContainerInspectResult.parse_obj(json_object) File "pydantic/main.py", line 520, in pydantic.main.BaseModel.parse_obj File "pydantic/main.py", line 362, in pydantic.main.BaseModel.init pydantic.error_wrappers.ValidationError: 1 validation error for ContainerInspectResult HostConfig -> Mounts -> 0 -> Source field required (type=value_error.missing)

raise ParsingError(

python_on_whales.client_config.ParsingError: There was an error parsing the json response from the Docker daemon. This is a bug with python-on-whales itself. Please head to https://github.com/gabrieldemarmiesse/python-on-whales/issues and open an issue. You should copy this error message and the json response from the Docker daemon. The json response was put in /tmp/tmphd1it3er.json because it's a bit too big to be printed on the screen. Make sure that there are no sensitive data in the json file before copying it in the github issue.

WolfgangFahl commented 3 years ago

really?

WolfgangFahl commented 3 years ago

Why would you have to choke like this on any json? Can't you just log a warning and return some reasonable result instead of shutting down the app with an exception?

WolfgangFahl commented 3 years ago

The json looks interesting and indeed as security relevant data like the MYSQL password in it - why does this go over the wire unencrypted?

WolfgangFahl commented 3 years ago

And https://github.com/gabrieldemarmiesse/python-on-whales/blob/master/python_on_whales/components/container/cli_wrapper.py assumes a stable json environment which is highly unlikely given the notorious incompatibility of docker versions.

WolfgangFahl commented 3 years ago
cat tmphd1it3er.json 
[
    {
        "Id": "39197bb1af1f12507f5df8bd17a69b8b7ec11f26360829d757e5d7faa3947311",
        "Created": "2021-06-20T14:56:15.937103778Z",
        "Path": "docker-entrypoint.sh",
        "Args": [
            "mysqld"
        ],
        "State": {
            "Status": "running",
            "Running": true,
            "Paused": false,
            "Restarting": false,
            "OOMKilled": false,
            "Dead": false,
            "Pid": 2468765,
            "ExitCode": 0,
            "Error": "",
            "StartedAt": "2021-06-20T14:56:18.587094927Z",
            "FinishedAt": "0001-01-01T00:00:00Z"
        },
        "Image": "sha256:104f4e7cbe8335d5a59bf4e85abe94402c46898ac01ba76f54e12350b077409b",
...
        "Name": "/mw1_27_7_db_1",
...
WolfgangFahl commented 3 years ago
def _get_inspect_result(self) -> ContainerInspectResult:
       """Only there to allow tools to know the return type"""
       return super()._get_inspect_result()

might need a try / catch to better handle errors.

WolfgangFahl commented 3 years ago

https://pydantic-docs.helpmanual.io/ is pedantic indeed which doesn't really help here.

When accessing the name of the container it is IMHO unnecessary that the rest of the JSON file is valid. IMHO it's the whole idea of JSON to be somewhat schemaless and not enforce to many rules ...

WolfgangFahl commented 3 years ago

This seems to be the problematic part:

"Mounts": [
                {
                    "Type": "volume",
                    "Target": "/var/lib/mysql"
                }
            ],

which is irrelevant for my usecase and in fact i am currently busy trying a workaround for the mariadb docker image which exactly seems to call for getting rid of the mounts. Now i am stuck between a rock and a hard place ...

WolfgangFahl commented 3 years ago
class Mount(DockerCamelModel):

            type: Optional[str]

            name: Optional[str]

            source: str

            destination: str

            driver: Optional[str]

            mode: str

            rw: bool

            <span class="pl-s1">propagation</span>: <span class="pl-s1">str</span>

source in this case must be optional.

gabrieldemarmiesse commented 3 years ago

I will take a look into it.

I would like to remind you that this software is free, and open source. I worked on it in my spare time. The license states that I am not responsible in case of dammage caused by this software. As such, you should be polite when submitting issues.

Statements like

really?

Why would you have to choke like this on any json?

If you claim your library works on macos, windows and linux you might want to test things.

Are not acceptables.

Furthermore, please make one big, structured bug report when opening an issue, rather than many small messages, which makes it hard to track what needs fixing.

After this being said, the bug report is valid and I'll look into how to make sure the json is parsed properly in this case.