Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Latest docker images fail to run blackd with an ImportError #4163

Closed
ravishi opened this issue Jan 22, 2024 · 8 comments · Fixed by #4357
Closed

Latest docker images fail to run blackd with an ImportError #4163

ravishi opened this issue Jan 22, 2024 · 8 comments · Fixed by #4357
Assignees
Labels
T: bug Something isn't working

Comments

@ravishi
Copy link

ravishi commented Jan 22, 2024

Describe the bug

I'm on MacOS. I try to run Black through Docker, but the container promptly stops with an ImportError related to the missing aiohttp dep.

To Reproduce

docker run -d -p 45484:45484 --pull always pyfound/black blackd --bind-host 0.0.0.0
# returns c42cfd39d8047d840beb779ea529005ef4d5682b060bcbca9b154e940bd346ab
docker logs -f c42cfd39d8047d840beb779ea529005ef4d5682b060bcbca9b154e940bd346ab
Traceback (most recent call last):
  File "/opt/venv/bin/blackd", line 5, in <module>
    from blackd import patched_main
  File "/opt/venv/lib/python3.12/site-packages/blackd/__init__.py", line 14, in <module>
    raise ImportError(
ImportError: aiohttp dependency is not installed: No module named 'aiohttp'. Please re-install black with the '[d]' extra install to obtain aiohttp_cors: `pip install black[d]`
  • Black's version: Any tag greater than pyfound/black:23.10.0
  • OS and Python version: [MacOS/14.2.1 (23C71)]
@ravishi ravishi added the T: bug Something isn't working label Jan 22, 2024
@ravishi ravishi changed the title Latest docker images Latest docker images fail with an ImportError Jan 22, 2024
@cooperlees cooperlees added the T: enhancement New feature or request label Jan 22, 2024
@cooperlees
Copy link
Collaborator

Howdy

We've on purpose never built the docker image with the [d] extra dependencies to keep the images as small as possible.

We could add this in - But I want to see what people prefer:

  • Add by default and make the image smaller (I have not calculated the size - But I expect it to be a small different)
    • Simple, but everyone occurs the size increase even if not using blackd
    • Image is currently around 55MB
    • aiohttp wheels seem to be around 1.5MB (but that's zip compressed)
  • Add a dedicated built for a extra [d] install and make new tags
    • A lot more complicated and more things to potentially fix in the future

Thoughts all?

@cooperlees
Copy link
Collaborator

cooperlees commented Jan 22, 2024

Actually, I am wrong, we do try to do the extra install:

https://github.com/psf/black/blob/main/Dockerfile#L13

I will look into it - But if anyone has time, PR welcoming fixing getting aiohttp into the container.

  • I would have expected it to be in the venv we copy over from the build container ...

@cooperlees cooperlees removed the T: enhancement New feature or request label Jan 22, 2024
@cooperlees cooperlees self-assigned this Jan 22, 2024
@JelleZijlstra
Copy link
Collaborator

I haven't delved into the history, but might be worth looking at what we changed in 23.10. Could be a Hatch bug; this reminds me of #4107 which is sort of the opposite.

@charleshan
Copy link

Confirmed that 23.10.0 is the last working version. The error occurs on 23.10.1 and onwards.

@cooperlees
Copy link
Collaborator

cooperlees commented May 10, 2024

I thought I'd try this to see if anything has changed and I was able to use the latest build + 24.4.2 from Docker hub and it works:

cooper@home1:~$ docker run --rm --mount type=bind,source="$(pwd)"/repos/black/src,target=/tmp/black_src pyfound/black:24.4.2 black /tmp
reformatted /tmp/black_src/black/handle_ipynb_magics.py

All done! ✨ 🍰 ✨
1 file reformatted, 40 files left unchanged.

Was surprised a formatting change happened too

FWIW it was

cooper@home1:~/repos/black$ git diff
diff --git a/src/black/handle_ipynb_magics.py b/src/black/handle_ipynb_magics.py
index 5b2847c..55ef226 100644
--- a/src/black/handle_ipynb_magics.py
+++ b/src/black/handle_ipynb_magics.py
@@ -17,30 +17,36 @@ else:
 from black.output import out
 from black.report import NothingChanged

-TRANSFORMED_MAGICS = frozenset((
-    "get_ipython().run_cell_magic",
-    "get_ipython().system",
-    "get_ipython().getoutput",
-    "get_ipython().run_line_magic",
-))
-TOKENS_TO_IGNORE = frozenset((
-    "ENDMARKER",
-    "NL",
-    "NEWLINE",
-    "COMMENT",
-    "DEDENT",
-    "UNIMPORTANT_WS",
-    "ESCAPED_NL",
-))
-PYTHON_CELL_MAGICS = frozenset((
-    "capture",
-    "prun",
-    "pypy",
-    "python",
-    "python3",
-    "time",
-    "timeit",
-))
+TRANSFORMED_MAGICS = frozenset(
+    (
+        "get_ipython().run_cell_magic",
+        "get_ipython().system",
+        "get_ipython().getoutput",
+        "get_ipython().run_line_magic",
+    )
+)
+TOKENS_TO_IGNORE = frozenset(
+    (
+        "ENDMARKER",
+        "NL",
+        "NEWLINE",
+        "COMMENT",
+        "DEDENT",
+        "UNIMPORTANT_WS",
+        "ESCAPED_NL",
+    )
+)
+PYTHON_CELL_MAGICS = frozenset(
+    (
+        "capture",
+        "prun",
+        "pypy",
+        "python",
+        "python3",
+        "time",
+        "timeit",
+    )
+)
 TOKEN_HEX = secrets.token_hex

Can some other people check but I think we're working?

cooper@home1:~$ docker run --rm --mount type=bind,source="$(pwd)"/repos/black/src,target=/tmp/black_src pyfound/black:24.4.2 black --version
black, 24.4.2 (compiled: yes)
Python (CPython) 3.12.3

@ravishi
Copy link
Author

ravishi commented May 10, 2024

Just tried 24.4.2 and got the same error:

$ docker run -p 45484:45484 -d --pull always pyfound/black:24.4.2 blackd --bind-host 0.0.0.0
24.4.2: Pulling from pyfound/black
22d97f6a5d13: Already exists
b41a1d042542: Already exists
c7f04f94ebc0: Already exists
15e2dabcf764: Already exists
95c793e4d75a: Already exists
e43703dac3bd: Pull complete
Digest: sha256:6b6dd2135dbb4606ab8bbc6e797fd576a3c4d8bd4fde3e402983049e99e19980
Status: Downloaded newer image for pyfound/black:24.4.2
20400e067008b754cf6172ed7e225b2f49a08e87249db3ebdea40bc7a947e376

$ docker ps -a
CONTAINER ID   IMAGE                                                                            COMMAND                  CREATED         STATUS                     PORTS                                                                     NAMES
20400e067008   pyfound/black:24.4.2                                                             "blackd --bind-host …"   5 seconds ago   Exited (1) 4 seconds ago                                                                             lucid_panini

$ docker logs 20400e067008
Traceback (most recent call last):
  File "/opt/venv/bin/blackd", line 5, in <module>
    from blackd import patched_main
  File "/opt/venv/lib/python3.12/site-packages/blackd/__init__.py", line 15, in <module>
    raise ImportError(
ImportError: aiohttp dependency is not installed: No module named 'aiohttp'. Please re-install black with the '[d]' extra install to obtain aiohttp_cors: `pip install black[d]`

@cooperlees, maybe you're getting different results because you're mounting the source folder?

@cooperlees
Copy link
Collaborator

Doh - I'm not testing blackd ... Silly me.

@cooperlees cooperlees changed the title Latest docker images fail with an ImportError Latest docker images fail to run blackd with an ImportError May 10, 2024
cooperlees pushed a commit that referenced this issue May 10, 2024
- Lets install black, then ask to install black with extrasC
  - pip sees black is installed and just installs extra dependencies

Test:
- Build local container
  - `docker build -t black_local .`
- Run blackd in container
  - `docker run -p 45484:45484 --rm black_local blackd --bind-host 0.0.0.0`
```
cooper@home1:~/repos/black$ docker run -p 45484:45484 --rm black_local blackd --bind-host 0.0.0.0
blackd version 24.4.3.dev11+gad60e62 listening on 0.0.0.0 port 45484
INFO:aiohttp.access:10.255.255.1 [10/May/2024:14:40:36 +0000] "GET / HTTP/1.1" 405 204 "-" "curl/8.5.0"

cooper@home1:~/repos/black$ curl http://10.6.9.2:45484
405: Method Not Allowed
```
- Test version is compiled
```
cooper@home1:~/repos/black$ docker run --rm black_local black --version
black, 24.4.3.dev11+gad60e62 (compiled: yes)
Python (CPython) 3.12.3
```

Fixes #4163
JelleZijlstra pushed a commit that referenced this issue May 10, 2024
…4357)

- Lets install black, then ask to install black with extrasC
  - pip sees black is installed and just installs extra dependencies

Test:
- Build local container
  - `docker build -t black_local .`
- Run blackd in container
  - `docker run -p 45484:45484 --rm black_local blackd --bind-host 0.0.0.0`
```
cooper@home1:~/repos/black$ docker run -p 45484:45484 --rm black_local blackd --bind-host 0.0.0.0
blackd version 24.4.3.dev11+gad60e62 listening on 0.0.0.0 port 45484
INFO:aiohttp.access:10.255.255.1 [10/May/2024:14:40:36 +0000] "GET / HTTP/1.1" 405 204 "-" "curl/8.5.0"

cooper@home1:~/repos/black$ curl http://10.6.9.2:45484
405: Method Not Allowed
```
- Test version is compiled
```
cooper@home1:~/repos/black$ docker run --rm black_local black --version
black, 24.4.3.dev11+gad60e62 (compiled: yes)
Python (CPython) 3.12.3
```

Fixes #4163
@cooperlees
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants