uranusjr opened a new issue #15815: URL: https://github.com/apache/airflow/issues/15815
I had this after reading #12537 and #9047. Currently `DockerOperator`’s `volumes` argument is passed directly to docker-py’s `bind` (aka `docker -v`). But `-v`’s behaviour has long been problematic, and [Docker has been pushing users to the new `--mount` syntax instead](https://docs.docker.com/storage/bind-mounts/#choose-the--v-or---mount-flag). With #12537, it seems like `-v`’s behaviour is also confusing to some Airflow users, so I want to migrate Airflow’s internals to `--mount`. However, `--mount` has a different syntax to `-v`, and the behaviour is also slightly different, so for compatibility reasons we can’t just do it under the hood. I can think of two possible solutions to this: A. Deprecate `volumes` altogether and introduce `DockerOperator(mounts=...)` This will emit a deprecation warning when the user passes `DockerOperator(volumes=...)` to tell them to convert to `DockerOperator(mounts=...)` instead. `volumes` will stay unchanged otherwise, and continue to be passed to bind mounts. `mounts` will take a list of [`docker.types.Mount`](https://docker-py.readthedocs.io/en/stable/api.html#docker.types.Mount) to describe the mounts. They will be passed directly to the mounts API. Some shorthands could be useful as well, for example: ```python DockerOperator( ... mounts=[ ('/root/data1', './data1'), # Source and target, default to volume mount. ('/root/data2', './data2', 'bind'), # Bind mount. ], ) ``` B. Reuse `volumes` and do introspection to choose between binds and mounts The `volumes` argument can be augmented to also accept `docker.types.Mount` instances, and internally we’ll do something like ```python binds = [] mounts = [] for vol in volumes: if isinstance(vol, str): binds.append(vol) elif isintance(vol, docker.types.Mount): mounts.append(vol) else: raise ValueError('...') if binds: warnings.warn('...', DeprecationWarning) ``` and pass the collected lists to binds and mounts respectively. I’m very interested in hearing thoughts on this. **Are you willing to submit a PR?** Yes **Related Issues** * #12537: Confusing on the bind syntax. * #9047: Implement mounting in `DockerSwarmOperator` (it’s a subclass of `DockerOperator`, but the `volumes` option is currently unused). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
