Make rules_docker compatible with Python 3?

[fedenusy]

Hello! When running rules_docker using Python 3, I get the following error:

ERROR: Analysis of target '//samwell:samwell_img' failed; build aborted: no such package '@go_image_base//image': Pull command failed: Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.3/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/Cellar/python/3.7.3/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/private/var/tmp/_bazel_fedenusy/82761fbb391b03a602abc1324ff91dda/external/puller/file/downloaded/__main__.py", line 30, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "/private/var/tmp/_bazel_fedenusy/82761fbb391b03a602abc1324ff91dda/external/puller/file/downloaded/containerregistry/client/__init__.py", line 23, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "/private/var/tmp/_bazel_fedenusy/82761fbb391b03a602abc1324ff91dda/external/puller/file/downloaded/containerregistry/client/docker_creds_.py", line 31, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 963, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 906, in _find_spec
  File "<frozen importlib._bootstrap_external>", line 1280, in find_spec
  File "<frozen importlib._bootstrap_external>", line 1254, in _get_spec
  File "<frozen importlib._bootstrap_external>", line 1235, in _legacy_get_spec
  File "<frozen importlib._bootstrap>", line 441, in spec_from_loader
  File "<frozen importlib._bootstrap_external>", line 594, in spec_from_file_location
  File "/private/var/tmp/_bazel_fedenusy/82761fbb391b03a602abc1324ff91dda/external/puller/file/downloaded/httplib2/__init__.py", line 988
    raise socket.error, msg
                      ^
SyntaxError: invalid syntax

From seeing this, I'm assuming rules_docker was written for Python 2.

The workaround I found is to:

• Set my machine's default python to python2
• Set --incompatible_use_python_toolchains
• Set --incompatible_py3_is_default=false

I think it should be simple to make rules_docker compatible with Python toolchains. We would just need to declare python_version = "PY2" somewhere within rules_docker... thoughts?

[fedenusy]

I looked around the rules_docker repo and it seems like every py_binary already declares python_version = "PY2"... so maybe it's not so simple?

[nlopezgi]

you probably need to add host_force_python=PY2 as exemplified in the bazelrc file: https://github.com/bazelbuild/rules_docker/blob/master/.bazelrc#L20
please let me know if this works as many of these new py toolchain flags are untested

[fedenusy]

To be clear, rules_docker works just fine with the following:

• Machine's default python set to python2
• --incompatible_use_python_toolchains
• --incompatible_py3_is_default=false

But since I need to declare the 2nd flag, I'm thinking rules_docker is incompatible with the new Python toolchains. If rules_docker were compatible, then I wouldn't need --incompatible_py3_is_default=false?

(The problem stems from the fact that internally we write Python 3. That flag forces us to declare python_version = "PY3" in all of our targets... we'd much rather keep the default at PY3 if at all possible.)

[nlopezgi]

Please read bazelbuild/bazel#7899 in detail for the status of python toolchains (see lots of comments about rules_docker there).
The tl;dr is that with incompatible_use_python_toolchains rules_docker needs to run with host_force_python=PY2 as we depend on a library that is not compatible with PY3 (and which we don't control) and its not possible in Bazel (now or in the past, in any way) to have a host config for both PY2 and PY3 as part of the same build.

So, you should be able to keep the default to PY3 (as long as you are not using PY3 as host tools), but you will need to use the host_force_python=PY2, or set python_version = "PY3" in all your targets that are only compatible with PY3 (which, imo, is not a bad thing). Please let me know if you have more questions.

[nlopezgi]

closing, please let me know if you'd like me to reopen for some reason

[qzmfranklin]

@fedenusy rules_docker can be patched to work with python3. I've managed it inside our team. Please let me know how much you are still interested in it.

[fedenusy]

@qzmfranklin yes I'm interested! How'd you pull it off?

[jdelfino]

In my opinion, this issue should not be closed. Currently, rules_docker can only be used by forcing all host tools to run under py2. We have host tools in our build that need to run under py3, so specifying host_force_python=PY2 is not an acceptable workaround for us.

I understand that fixing this is not trivial, but python2 retires in 6 months, so it will need to be dealt with somehow, at some point soon, right?

[nlopezgi]

@jdelfino
Thanks, for your comment
host_force_python=PY2 is indeed a bad workaround, and indeed it blocks use of host tools that require PY3, but its all we have from Bazel atm. Also, iiuc, this is not a regression, it has never been possible to have host tools that build with PY2 and PY3 to be used as part of the same Bazel build.

Also, yes, we are dealing with it (just in a different way, thus the issue being closed). We are currently working on migrating to use go containerregistry. Please keep track of PRs landing soon if you are interested in this.

[jdelfino]

Thanks @nlopezgi! I don't believe this is a regression. For anyone else that ends up here, I found #580, which appears to track the migration to go containerregistry.

[ittaiz]

@nlopezgi is this solely because rules_docker depends on python containerregistry which is not actively maintained? When rules_docker will move to go containerregistry will this be resolved as far as rules_docker is concerned?

[nlopezgi]

Hi @ittaiz, yes this should be resolved once we migrate off python containerregistry

[mickael-carl]

What about keeping this open to track actually this problem, until the move to go containerregistry is complete? That seems more sensible than closing imho, as this issue is something that will pop up again until the change lands, and --host_force_python=PY2 is just a workaround as you point out.

[nlopezgi]

Reopening just to make it easier for folks to find this issue, but --host_force_python=PY2 is the only workaround until we have either a better solution from Bazel team (see bazelbuild/bazel#7899) or migration to go containerregistry lands

[fejta]

If rules_docker requires --host_force_python=PY2 please add that to https://github.com/bazelbuild/rules_docker#setup

[nlopezgi]

thanks @fejta, added to Setup in #953.

[c4urself]

I've got a Python-3 compatible branch that works for basic things for me, could use some help testing, see: google/containerregistry#109 (comment)

[nlopezgi]

Hi @c4urself ,
Thanks for working on this. The main challenge you will have getting these changes in is that py containerregistry is not really taking too many community contributions atm, iiuc. The solution we are working on for this is to migrate off py containerregistry. Please follow #580 for status of that effort.

[c4urself]

@nlopezgi -- understood, looking forward to the Golang stuff getting finished. I've got a working version of rules_docker under Python2 and Python3 by simply changing the containerregistry rules to a git_repository pointing to a forked branch: c4urself@860e165 -- how crazy would it be to start using that git_repository in the official rules_docker and giving people Python-3 compatibility while we wait for #580?

[nlopezgi]

@nlopezgi -- understood, looking forward to the Golang stuff getting finished. I've got a working version of rules_docker under Python2 and Python3 by simply changing the containerregistry rules to a git_repository pointing to a forked branch: c4urself@860e165 -- how crazy would it be to start using that git_repository in the official rules_docker and giving people Python-3 compatibility while we wait for #580?

too crazy for me (plus there's technical reasons why that would be a nightmare for us to maintain). I really don't want to make this situation more complex by adding another source of dependency / variability. But thanks for the suggestion!

[fejta]

Do we have an ETA for the golang conversion? Can we convince containerregistry to accept the contribution? It is definitely annoying to simultaneously default bazel to py3 while having core rules like rules_docker not support py3.

[nlopezgi]

status has been updated in #580. I would love for containerregistry to take contributions, but it seems the team in charge has other properties atm. As for changing the bazel default, that's a Bazel 1.0 decision that is probably hard to roll back. We appreciate your patience and hope to finish the migration soon.

[aiuto]

Less extreme idea.....
We work with the containerregistry team and c4urself to allow contributions to a "py3" branch of the that repo. The premise being

• that someone from rules_docker or bazel takes responsibility for merges into that branch only. This mitigates their resposibilty for making sure submissions do not break existing users.
• that we produce a single release
• that this release is what rule_docker depends on for the N?-months until it switches to go-containerregistry

[c4urself]

Sure, let me know, happy to work with whoever, for now I'm going to use my Python-3 compatible fork of rules_docker, and by the way these rules are great, loving it so far.

[nlopezgi]

Less extreme idea.....
We work with the containerregistry team and c4urself to allow contributions to a "py3" branch of the that repo. The premise being

• that someone from rules_docker or bazel takes responsibility for merges into that branch only. This mitigates their resposibilty for making sure submissions do not break existing users.
• that we produce a single release
• that this release is what rule_docker depends on for the N?-months until it switches to go-containerregistry

The main issue we have that makes these alternatives challenging is that the SoT for containerregistry is an internal google repo, and rules_docker needs to maintain compliance with this internal version to support internal use cases (even though rules_docker SoT is github). Changing the dep in rules_docker to something else would mean, for us, that the automation we have set in place to sync our SoT (in github) with internal repos would likely require manual intervention at each change to make sure discrepancies between SoT and internal rules do not cause breakages. Moreover, since our SoT is github, our test coverage is mostly here, so breaking internal users unintentionally would be a common occurrence.

[smukherj1]

Most of the problems mentioned in this issue should now be fixed because rules_docker python binaries have been migrated to Go binaries that depend on go-containerregistry. See #580.

[fedenusy]

🎉 🙏