zoni / mkdocs-diagrams

MkDocs plugin to render Diagrams files
MIT License
27 stars 3 forks source link

no module named 'diagrams' when trying to build docs #2

Closed decastromonteiro closed 4 years ago

decastromonteiro commented 4 years ago

Hello, when I try to mkdocs build, I get the following error.

If I try to execute the script by itself, or import diagrams on a Python console, it works just fine.

Any clue of what it can be?

Traceback (most recent call last):
  File "example.diagrams.py", line 1, in <module>
    from diagrams import Cluster, Diagram
ModuleNotFoundError: No module named 'diagrams'
ERROR   -  Worker raised an exception while rendering a diagram
decastromonteiro commented 4 years ago

Did additional verifications and it seems that the problem is that I'm using venv to run mkdocs, but the plugin is trying to use another python enviroment, not the one I'm using.

Ran it on gitlab pages and it was fine. Any idea on how to fix this issue?

zoni commented 4 years ago

Can you share a little bit of detail as to how you're starting mkdocs under that venv?

This plugin just calls python using subproccess, which you can see here: https://github.com/zoni/mkdocs-diagrams/blob/9afe237e8a842a910372aa8d0db6a4c72e263c46/mkdocs_diagrams/plugin.py#L63

This should be inheriting the environment that mkdocs was started with, so it should "just work" if the python binary of your venv comes first on your $PATH.

decastromonteiro commented 4 years ago

Yes, the venv path comes first on the enviroment

(npi) PS C:\Users\ledecast\PycharmProjects\npi> $env:path -split ";"
C:\Users\ledecast\OneDrive - Leo\Projetos\Python\PythonEnvironments\npi\Scripts
C:\WINDOWS\system32
C:\WINDOWS
C:\WINDOWS\System32\Wbem
C:\WINDOWS\System32\WindowsPowerShell\v1.0\
C:\WINDOWS\System32\OpenSSH\
C:\Program Files\Git\cmd
C:\ProgramData\chocolatey\bin
C:\Users\ledecast\AppData\Local\Programs\Python\Python38\Scripts\
C:\Users\ledecast\AppData\Local\Programs\Python\Python38\
C:\Users\ledecast\AppData\Local\Programs\Python\Python37\Scripts\
C:\Users\ledecast\AppData\Local\Programs\Python\Python37\
C:\Users\ledecast\AppData\Local\Microsoft\WindowsApps
C:\Program Files\JetBrains\PyCharm 2020.1.1\bin

C:\Users\ledecast\AppData\Local\Programs\MiKTeX 2.9\miktex\bin\x64\
C:\Users\ledecast\AppData\Local\Programs\Microsoft VS Code\bin

I'm running it on Windows, python 37.

Basically this is the entire traceback I get.

(npi) PS C:\Users\ledecast\PycharmProjects\npi> mkdocs build
[MERMAID] {}
Traceback (most recent call last):
  File "example.diagrams.py", line 1, in <module>
    from diagrams import Cluster, Diagram
ModuleNotFoundError: No module named 'diagrams'
ERROR   -  Worker raised an exception while rendering a diagram
Traceback (most recent call last):
  File "c:\users\ledecast\onedrive - leo\projetos\python\pythonenvironments\npi\lib\site-packages\mkdocs_diagrams\plugin.py", line 75, in _walk_files_and_render
    job.result()
  File "C:\Users\ledecast\AppData\Local\Programs\Python\Python37\lib\concurrent\futures\_base.py", line 428, in result
    return self.__get_result()
  File "C:\Users\ledecast\AppData\Local\Programs\Python\Python37\lib\concurrent\futures\_base.py", line 384, in __get_result
    raise self._exception
  File "C:\Users\ledecast\AppData\Local\Programs\Python\Python37\lib\concurrent\futures\thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "c:\users\ledecast\onedrive - leo\projetos\python\pythonenvironments\npi\lib\site-packages\mkdocs_diagrams\plugin.py", line 63, in _render_diagram
    subprocess.run(["python", filename], check=True, cwd=dest_dir)
  File "C:\Users\ledecast\AppData\Local\Programs\Python\Python37\lib\subprocess.py", line 512, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['python', 'example.diagrams.py']' returned non-zero exit status 1.
INFO    -  Cleaning site directory
INFO    -  Building documentation to directory: C:\Users\ledecast\PycharmProjects\npi\public
INFO    -  The following pages exist in the docs directory, but are not included in the "nav" configuration:
  - guides\cmg\host_setup.md
  - guides\cmg\kvm\db-vm.md
WARNING -  Documentation file 'index.md' contains a link to 'event_processing.png' which is not found in the documentation files.
INFO    -  Documentation built in 2.88 seconds
zoni commented 4 years ago

According to https://stackoverflow.com/questions/5658622/python-subprocess-popen-environment-path and https://stackoverflow.com/questions/41860668/why-does-this-python-subprocess-command-only-work-when-shell-true-on-windows, the way executables are looked up differs on Windows, which I suspect is what's going on here.

To confirm my hunch, could you change the following line: https://github.com/zoni/mkdocs-diagrams/blob/9afe237e8a842a910372aa8d0db6a4c72e263c46/mkdocs_diagrams/plugin.py#L63

From:

subprocess.run(["python", filename], check=True, cwd=dest_dir)

To:

subprocess.run(["python", filename], check=True, cwd=dest_dir, shell=True)

In your copy of this plugin (c:\users\ledecast\onedrive - leo\projetos\python\pythonenvironments\npi\lib\site-packages\mkdocs_diagrams\plugin.py if I'm reading your Traceback right) and see if it works then?

decastromonteiro commented 4 years ago

Thanks for help @zoni , shell=True fixed it.

Is there any special reason not to use shell=True on the original code?

zoni commented 4 years ago

Using shell=True means a shell process is used to launch the command specified. Unless you specifically want/need shell semantics/logic, doing so is just adding the overhead of spawning an additional process at best, and opening up a security vulnerability at worst (due to improper escaping of input/arguments).

Could you try one other variation for me please? Instead of using shell=True, could you try changing that line to:

import sys; subprocess.run([sys.executable, filename], check=True, cwd=dest_dir, shell=False)

I'm not 100% sure that will work, but if it does, we'd have a nice solution that wouldn't require the use of a shell.

decastromonteiro commented 4 years ago

It also worked Zoni. 💯

plugin.py looks like this:

import concurrent.futures
import logging
import os
import shutil
import subprocess
import time
import sys

import mkdocs
import mkdocs.plugins

from mkdocs.structure.files import get_files

# This global is a hack to keep track of the last time the plugin rendered diagrams.
# A global is required because plugins are reinitialized each time a change is detected.
last_run_timestamp = 0

class DiagramsPlugin(mkdocs.plugins.BasePlugin):
    """
    A MkDocs plugin to render Diagrams files.

    See also https://diagrams.mingrammer.com/.
    """

    config_scheme = (
        (
            "file_extension",
            mkdocs.config.config_options.Type(str, default=".diagrams.py"),
        ),
        ("max_workers", mkdocs.config.config_options.Type(int, default=None)),
    )

    def __init__(self):
        self.log = logging.getLogger("mkdocs.plugins.diagrams")
        self.pool = None

    def _create_threadpool(self):
        max_workers = self.config["max_workers"]
        if max_workers is None:
            max_workers = os.cpu_count() + 2
        self.log.debug(
            "Using up to %d concurrent workers for diagrams rendering", max_workers
        )
        return concurrent.futures.ThreadPoolExecutor(max_workers=max_workers)

    def _render_diagram(self, file):
        self.log.debug(f"Rendering {file.name}")
        # The two commented lines below would build in the destination
        # (site_dir) directory instead of the original source directory.
        # Unfortunately this results in incorrect image URLs (they don't get
        # rewritten to the proper relative path one directory up).
        #
        # dest_dir = os.path.dirname(file.abs_dest_path)
        # filename = file.abs_dest_path[len(dest_dir)+1:]
        dest_dir = os.path.dirname(file.abs_src_path)
        filename = file.abs_src_path[len(dest_dir) + 1 :]

        # Even when writing in abs_src_path rather than abs_dest_path, this
        # seems needed to make livereload accurately pick up changes.
        os.makedirs(os.path.dirname(file.abs_dest_path), exist_ok=True)
        shutil.copy(file.abs_src_path, file.abs_dest_path)

        subprocess.run([sys.executable, filename], check=True, cwd=dest_dir, shell=False)

    def _walk_files_and_render(self, config):
        pool = self._create_threadpool()
        files = get_files(config)
        jobs = []
        for file in files:
            if file.src_path.endswith(self.config["file_extension"]):
                jobs.append(pool.submit(self._render_diagram, file))

        for job in concurrent.futures.as_completed(jobs):
            try:
                job.result()
            except Exception:
                self.log.exception("Worker raised an exception while rendering a diagram")

    def on_pre_build(self, config):
        global last_run_timestamp
        if int(time.time()) - last_run_timestamp < 10:
            self.log.info(
                "Watcher started looping, skipping diagrams rendering on this run"
            )
            return
        self._walk_files_and_render(config)
        last_run_timestamp = int(time.time())
zoni commented 4 years ago

I just released v1.0.0 on PyPI with the proper fix for this in it, so next time you install/upgrade, you shouldn't have to do any more manual patching. Thanks for the excellent feedback here while we figured out the issue.