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.

Sign up for GitHub

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

Ensure npm install runs once at a time #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Ensure npm install runs once at a time #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions lektor_webpack_support.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import threading

from lektor.pluginsystem import Plugin
from lektor.reporter import reporter
Expand All @@ -11,6 +12,7 @@ class WebpackSupportPlugin(Plugin):

def __init__(self, *args, **kwargs):
Plugin.__init__(self, *args, **kwargs)
self.npm_lock = threading.Lock()
self.webpack_process = None

def is_enabled(self, extra_flags):
Expand All @@ -24,9 +26,13 @@ def run_webpack(self, watch=False):
return portable_popen(args, cwd=webpack_root)

def npm_install(self):
reporter.report_generic('Running npm install')
webpack_root = os.path.join(self.env.root_path, 'webpack')
portable_popen(['npm', 'install'], cwd=webpack_root).wait()
if self.npm_lock.acquire(False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you specify blocking=False here? I had to look up the documentation for threading.Lock to understand what this False was for.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, no:

>>> import threading
>>> threading.Lock().acquire(blocking=False)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: acquire() takes no keyword arguments

Would an inline comment be useful?

if self.npm_lock.acquire(False):  # non-blocking

reporter.report_generic('Running npm install')
webpack_root = os.path.join(self.env.root_path, 'webpack')
portable_popen(['npm', 'install'], cwd=webpack_root).wait()
else:
self.npm_lock.acquire()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you acquiring the lock here, if you're not doing anything with it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To maintain the behavior that this method does not exit until the npm install process is complete, since callers expect to have a complete set of installed dependencies after calling it. (I.e., if you can acquire the lock, run npm and then release it. If you can't acquire the lock, wait for it to be released before exiting.) Otherwise you might end up attempting to run webpack before it's installed.

self.npm_lock.release()

def on_server_spawn(self, **extra):
extra_flags = extra.get("extra_flags") or extra.get("build_flags") or {}
Expand Down
18 changes: 18 additions & 0 deletions tests/test_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import py
import threading
import time


def test_disabled_by_default(plugin):
Expand Down Expand Up @@ -66,3 +68,19 @@ def test_server_stop(plugin, mocker):
plugin.webpack_process = mocker.Mock()
plugin.on_server_stop()
plugin.webpack_process.kill.assert_called_with()


def test_single_npm_process(plugin, mocker):
def pause(*args, **kwargs):
time.sleep(.1)
return mocker.DEFAULT

mock_popen = mocker.patch("lektor_webpack_support.portable_popen")
mock_popen.side_effect = pause
thread1 = threading.Thread(target=plugin.npm_install)
thread2 = threading.Thread(target=plugin.npm_install)
thread1.start()
thread2.start()
thread1.join()
thread2.join()
assert mock_popen.call_count == 1