You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In node/docker.py the docstring describes the volume parameter as follows:
volumes : list of dict or list of str
A dictionary of volumes. Each entry has a name or (absolute) path as key
and settings or a absolute path inside the container as value. See :code:`examples/volumes_and_ports.py`.
There are multiple issues with:
The referenced example is no longer existing. The example was deleted with the following commit. I'd suggest to restore this or expand the docstring or documentation
The docstring defines the parameter to be either a list of dict or a list of str but during the init function the referencing statement expects it to be a dict since its using the items() function like this:
self.volumes = dict(map(expand_volume_shorthand, volumes.items())) if volumes else None
using the expand_volume_shorthand function.
I checked with the documentation of the docker python library and i don't really understand the purpose of the function other then to provide default for the mounting mode. The library accepts lists as well as dicts and i can't see a reason to omit the custom function and directly relay the parameter to the docker lib.
If there are no objections, i would create a PR that gets rid of the "expand_volume_shorthand" function and parsing of the parameter. Instead I would pass through the parameter and adapt the docstring accordingly.
The text was updated successfully, but these errors were encountered:
Ok i guess i found the reason why the volumes_and_ports example got deleted. It doesn't really work with Marvis running inside a docker while being connected to the docker daemon on the host.
The example uses os.path.dirname to get the directory of the scenario file and uses that to build a absolute path. But all of that happens inside the container and then the absolute path gets passed to the docker daemon running outside of the container.
This feels like it should be documented as a limitation of running marvis inside a docker.
In node/docker.py the docstring describes the volume parameter as follows:
There are multiple issues with:
items()
function like this:using the expand_volume_shorthand function.
I checked with the documentation of the docker python library and i don't really understand the purpose of the function other then to provide default for the mounting mode. The library accepts lists as well as dicts and i can't see a reason to omit the custom function and directly relay the parameter to the docker lib.
If there are no objections, i would create a PR that gets rid of the "expand_volume_shorthand" function and parsing of the parameter. Instead I would pass through the parameter and adapt the docstring accordingly.
The text was updated successfully, but these errors were encountered: