Repository: streamlit/streamlit
Pull request: LocalSourcesWatcher: blacklist venv and hidden folders
LocalSourcesWatcher blacklists conda folders (anaconda2/3, miniconda2/3) and any folder whose name starts with a `.`.

For simplicity, the blacklist syntax now supports Unix glob patterns.

(Also, there are a few unrelated tests that I cleaned up while I was fixing the new tests that were written for this PR.)

Fixes #129

Problem statement:
Blacklist common virtualenv folders by default
We should blacklist the folders below from being watched by Streamlit. This would fix the issue where some people hit the inotify watch limit when running Streamlit from a weird working directory.

* */.virtualenv
* */.venv
* */anaconda3
* */anaconda2
* */miniconda3
* */miniconda2
* Actually let's blacklist every hidden folder: `.*`


See also the config option `server.folderWatchBlacklist`. For this fix, you can probably use the same mechanism this config option uses.

Hints from issue discussion:

Commit message: Tweak

Patch to review:
diff --git a/lib/streamlit/watcher/LocalSourcesWatcher.py b/lib/streamlit/watcher/LocalSourcesWatcher.py
index 33080a28d7cf..236c9773b4dc 100644
--- a/lib/streamlit/watcher/LocalSourcesWatcher.py
+++ b/lib/streamlit/watcher/LocalSourcesWatcher.py
@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import fnmatch
 import os
 import sys
 import collections
@@ -28,6 +29,7 @@
 from streamlit import util
 
 from streamlit.logger import get_logger
+
 LOGGER = get_logger(__name__)
 
 try:
@@ -39,15 +41,18 @@
     # Fallback that doesn't use watchdog.
     from streamlit.watcher.PollingFileWatcher import PollingFileWatcher as FileWatcher
 
-    if not config.get_option('global.disableWatchdogWarning'):
-        msg = '\n  $ xcode-select --install' if util.is_darwin() else ''
+    if not config.get_option("global.disableWatchdogWarning"):
+        msg = "\n  $ xcode-select --install" if util.is_darwin() else ""
 
-        LOGGER.warning("""
+        LOGGER.warning(
+            """
   For better performance, install the Watchdog module:
   %s
   $ pip install watchdog
 
-        """ % msg)
+        """
+            % msg
+        )
 
 
 WatchedModule = collections.namedtuple("WatchedModule", ["watcher", "module_name"])
@@ -61,6 +66,11 @@ def __init__(self, report, on_file_changed):
 
         self._folder_blacklist = config.get_option("server.folderWatchBlacklist")
 
+        # Blacklist some additional folders, using glob syntax.
+        self._folder_blacklist.extend(
+            ["**/.*", "**/anaconda2", "**/anaconda3", "**/miniconda2", "**/miniconda3"]
+        )
+
         # A dict of filepath -> WatchedModule.
         self._watched_modules = {}
 
@@ -140,12 +150,12 @@ def update_watched_modules(self):
                     # .origin is 'built-in'.
                     continue
 
-                folder_is_blacklisted = any(
+                is_in_blacklisted_folder = any(
                     _file_is_in_folder(filepath, blacklisted_folder)
                     for blacklisted_folder in self._folder_blacklist
                 )
 
-                if folder_is_blacklisted:
+                if is_in_blacklisted_folder:
                     continue
 
                 file_is_new = filepath not in self._watched_modules
@@ -175,7 +185,10 @@ def update_watched_modules(self):
                 self._deregister_watcher(filepath)
 
 
-def _file_is_in_folder(filepath, folderpath):
-    # Assumes filepath is an absolute path, as a teeny tiny optimization.
-    folderpath = os.path.abspath(folderpath) + "/"
-    return filepath.startswith(folderpath)
+def _file_is_in_folder(filepath, folderpath_glob):
+    # Strip trailing slash if it exists
+    if folderpath_glob.endswith("/"):
+        folderpath_glob = folderpath_glob[:-1]
+
+    file_dir = os.path.dirname(filepath)
+    return fnmatch.fnmatch(file_dir, folderpath_glob)
diff --git a/lib/tests/streamlit/delta_generator_test.py b/lib/tests/streamlit/delta_generator_test.py
index f3c53593f820..f17b426b99b0 100644
--- a/lib/tests/streamlit/delta_generator_test.py
+++ b/lib/tests/streamlit/delta_generator_test.py
@@ -150,9 +150,9 @@ def test_wraps_with_cleaned_sig(self):
         wrapped = wrapped_function.keywords.get("wrapped")
 
         # Check meta data.
-        self.assertEqual(wrapped.__module__, "delta_generator_test")
-        self.assertEqual(wrapped.__name__, "fake_text")
-        self.assertEqual(wrapped.__doc__, "Fake text delta generator.")
+        self.assertEqual("delta_generator_test", wrapped.__module__)
+        self.assertEqual("fake_text", wrapped.__name__)
+        self.assertEqual("Fake text delta generator.", wrapped.__doc__)
 
         # Verify original signature
         sig = signature(FakeDeltaGenerator.fake_text)
diff --git a/lib/tests/streamlit/help_test.py b/lib/tests/streamlit/help_test.py
index 40ca5910d9b1..510c4a94909e 100644
--- a/lib/tests/streamlit/help_test.py
+++ b/lib/tests/streamlit/help_test.py
@@ -43,14 +43,14 @@ def my_func(some_param, another_param=123):
         st.help(my_func)
 
         ds = self.get_delta_from_queue().new_element.doc_string
-        self.assertEqual(ds.name, "my_func")
-        self.assertEqual(ds.module, "help_test")
+        self.assertEqual("my_func", ds.name)
+        self.assertEqual("help_test", ds.module)
         if is_python_2:
-            self.assertEqual(ds.type, "<type 'function'>")
+            self.assertEqual("<type 'function'>", ds.type)
         else:
-            self.assertEqual(ds.type, "<class 'function'>")
-        self.assertEqual(ds.signature, "(some_param, another_param=123)")
-        self.assertEqual(ds.doc_string, "This is the doc")
+            self.assertEqual("<class 'function'>", ds.type)
+        self.assertEqual("(some_param, another_param=123)", ds.signature)
+        self.assertEqual("This is the doc", ds.doc_string)
 
     def test_basic_func_without_doc(self):
         """Test basic function without docstring."""
@@ -61,14 +61,14 @@ def my_func(some_param, another_param=123):
         st.help(my_func)
 
         ds = self.get_delta_from_queue().new_element.doc_string
-        self.assertEqual(ds.name, "my_func")
-        self.assertEqual(ds.module, "help_test")
+        self.assertEqual("my_func", ds.name)
+        self.assertEqual("help_test", ds.module)
         if is_python_2:
-            self.assertEqual(ds.type, "<type 'function'>")
+            self.assertEqual("<type 'function'>", ds.type)
         else:
-            self.assertEqual(ds.type, "<class 'function'>")
-        self.assertEqual(ds.signature, "(some_param, another_param=123)")
-        self.assertEqual(ds.doc_string, "No docs available.")
+            self.assertEqual("<class 'function'>", ds.type)
+        self.assertEqual("(some_param, another_param=123)", ds.signature)
+        self.assertEqual("No docs available.", ds.doc_string)
 
     def test_deltagenerator_func(self):
         """Test Streamlit DeltaGenerator function."""
@@ -76,14 +76,14 @@ def test_deltagenerator_func(self):
         st.help(st.audio)
 
         ds = self.get_delta_from_queue().new_element.doc_string
-        self.assertEqual(ds.name, "audio")
-        self.assertEqual(ds.module, "streamlit")
+        self.assertEqual("audio", ds.name)
+        self.assertEqual("streamlit", ds.module)
         if is_python_2:
-            self.assertEqual(ds.type, "<type 'function'>")
-            self.assertEqual(ds.signature, "(data, format=u'audio/wav')")
+            self.assertEqual("<type 'function'>", ds.type)
+            self.assertEqual("(data, format=u'audio/wav')", ds.signature)
         else:
-            self.assertEqual(ds.type, "<class 'function'>")
-            self.assertEqual(ds.signature, "(data, format='audio/wav')")
+            self.assertEqual("<class 'function'>", ds.type)
+            self.assertEqual("(data, format='audio/wav')", ds.signature)
         self.assertTrue(ds.doc_string.startswith("Display an audio player"))
 
     def test_unwrapped_deltagenerator_func(self):
@@ -91,13 +91,13 @@ def test_unwrapped_deltagenerator_func(self):
         st.help(st.dataframe)
 
         ds = self.get_delta_from_queue().new_element.doc_string
-        self.assertEqual(ds.name, "dataframe")
-        self.assertEqual(ds.module, "streamlit")
+        self.assertEqual("dataframe", ds.name)
+        self.assertEqual("streamlit", ds.module)
         if is_python_2:
-            self.assertEqual(ds.type, "<type 'function'>")
+            self.assertEqual("<type 'function'>", ds.type)
         else:
-            self.assertEqual(ds.type, "<class 'function'>")
-        self.assertEqual(ds.signature, "(data=None, width=None, height=None)")
+            self.assertEqual("<class 'function'>", ds.type)
+        self.assertEqual("(data=None, width=None, height=None)", ds.signature)
         self.assertTrue(ds.doc_string.startswith("Display a dataframe"))
 
     def test_st_cache(self):
@@ -105,12 +105,12 @@ def test_st_cache(self):
         st.help(st.cache)
 
         ds = self.get_delta_from_queue().new_element.doc_string
-        self.assertEqual(ds.name, "cache")
-        self.assertEqual(ds.module, "streamlit")
+        self.assertEqual("cache", ds.name)
+        self.assertEqual("streamlit", ds.module)
         if is_python_2:
-            self.assertEqual(ds.type, "<type 'function'>")
+            self.assertEqual("<type 'function'>", ds.type)
         else:
-            self.assertEqual(ds.type, "<class 'function'>")
+            self.assertEqual("<class 'function'>", ds.type)
             self.assertEqual(
                 ds.signature,
                 ("(func=None, persist=False, " "ignore_hash=False, show_spinner=True)"),
@@ -122,13 +122,13 @@ def test_st_write(self):
         st.help(st.write)
 
         ds = self.get_delta_from_queue().new_element.doc_string
-        self.assertEqual(ds.name, "write")
-        self.assertEqual(ds.module, "streamlit")
+        self.assertEqual("write", ds.name)
+        self.assertEqual("streamlit", ds.module)
         if is_python_2:
-            self.assertEqual(ds.type, "<type 'function'>")
+            self.assertEqual("<type 'function'>", ds.type)
         else:
-            self.assertEqual(ds.type, "<class 'function'>")
-        self.assertEqual(ds.signature, "(*args, **kwargs)")
+            self.assertEqual("<class 'function'>", ds.type)
+        self.assertEqual("(*args, **kwargs)", ds.signature)
         self.assertTrue(ds.doc_string.startswith("Write arguments to the"))
 
     def test_builtin_func(self):
@@ -136,14 +136,14 @@ def test_builtin_func(self):
         st.help(dir)
 
         ds = self.get_delta_from_queue().new_element.doc_string
-        self.assertEqual(ds.name, "dir")
+        self.assertEqual("dir", ds.name)
         if is_python_2:
-            self.assertEqual(ds.module, "__builtin__")
-            self.assertEqual(ds.type, "<type 'builtin_function_or_method'>")
+            self.assertEqual("__builtin__", ds.module)
+            self.assertEqual("<type 'builtin_function_or_method'>", ds.type)
         else:
-            self.assertEqual(ds.module, "builtins")
-            self.assertEqual(ds.type, "<class 'builtin_function_or_method'>")
-        self.assertEqual(ds.signature, "")
+            self.assertEqual("builtins", ds.module)
+            self.assertEqual("<class 'builtin_function_or_method'>", ds.type)
+        self.assertEqual("", ds.signature)
         self.assertTrue(len(ds.doc_string) > 0)
 
     def test_builtin_obj(self):
@@ -151,13 +151,13 @@ def test_builtin_obj(self):
         st.help(123)
 
         ds = self.get_delta_from_queue().new_element.doc_string
-        self.assertEqual(ds.name, "")
-        self.assertEqual(ds.module, "")
+        self.assertEqual("", ds.name)
+        self.assertEqual("", ds.module)
         if is_python_2:
-            self.assertEqual(ds.type, "<type 'int'>")
+            self.assertEqual("<type 'int'>", ds.type)
         else:
-            self.assertEqual(ds.type, "<class 'int'>")
-        self.assertEqual(ds.signature, "")
+            self.assertEqual("<class 'int'>", ds.type)
+        self.assertEqual("", ds.signature)
         self.assertTrue(len(ds.doc_string) > 0)
 
     def test_doc_defined_for_type(self):
@@ -171,7 +171,7 @@ def test_doc_defined_for_type(self):
         st.help(array)
 
         ds = self.get_delta_from_queue().new_element.doc_string
-        self.assertEqual(ds.name, "")
+        self.assertEqual("", ds.name)
         self.assertTrue("ndarray" in ds.doc_string)
 
     def test_doc_type_is_type(self):
@@ -185,6 +185,6 @@ class MyClass(object):
 
         ds = self.get_delta_from_queue().new_element.doc_string
         self.assertEqual(type(MyClass), type)
-        self.assertEqual(ds.name, "MyClass")
-        self.assertEqual(ds.module, "help_test")
-        self.assertEqual(ds.doc_string, "No docs available.")
+        self.assertEqual("MyClass", ds.name)
+        self.assertEqual("help_test", ds.module)
+        self.assertEqual("No docs available.", ds.doc_string)
diff --git a/lib/tests/streamlit/watcher/LocalSourcesWatcher_test.py b/lib/tests/streamlit/watcher/LocalSourcesWatcher_test.py
index 9f35772a5298..28a7f9f793f2 100644
--- a/lib/tests/streamlit/watcher/LocalSourcesWatcher_test.py
+++ b/lib/tests/streamlit/watcher/LocalSourcesWatcher_test.py
@@ -24,20 +24,42 @@
 from streamlit import config
 from streamlit.Report import Report
 from streamlit.watcher import LocalSourcesWatcher
+from streamlit.watcher.LocalSourcesWatcher import _file_is_in_folder
 
 
 class FileIsInFolderTest(unittest.TestCase):
     def test_file_in_folder(self):
+        # Test with and without trailing slash
         ret = LocalSourcesWatcher._file_is_in_folder("/a/b/c/foo.py", "/a/b/c/")
         self.assertTrue(ret)
+        ret = LocalSourcesWatcher._file_is_in_folder("/a/b/c/foo.py", "/a/b/c")
+        self.assertTrue(ret)
 
     def test_file_not_in_folder(self):
+        # Test with and without trailing slash
         ret = LocalSourcesWatcher._file_is_in_folder("/a/b/c/foo.py", "/d/e/f/")
         self.assertFalse(ret)
+        ret = LocalSourcesWatcher._file_is_in_folder("/a/b/c/foo.py", "/d/e/f")
+        self.assertFalse(ret)
 
     def test_rel_file_not_in_folder(self):
+        # Test with and without trailing slash
         ret = LocalSourcesWatcher._file_is_in_folder("foo.py", "/d/e/f/")
         self.assertFalse(ret)
+        ret = LocalSourcesWatcher._file_is_in_folder("foo.py", "/d/e/f")
+        self.assertFalse(ret)
+
+    def test_file_in_folder_glob(self):
+        ret = LocalSourcesWatcher._file_is_in_folder("/a/b/c/foo.py", "**/c")
+        self.assertTrue(ret)
+
+    def test_file_not_in_folder_glob(self):
+        ret = LocalSourcesWatcher._file_is_in_folder("/a/b/c/foo.py", "**/f")
+        self.assertFalse(ret)
+
+    def test_rel_file_not_in_folder_glob(self):
+        ret = LocalSourcesWatcher._file_is_in_folder("foo.py", "**/f")
+        self.assertFalse(ret)
 
 
 if sys.version_info[0] == 2:
@@ -51,7 +73,7 @@ def test_rel_file_not_in_folder(self):
 
 REPORT_PATH = os.path.join(os.path.dirname(__file__), "test_data/not_a_real_script.py")
 REPORT = Report(REPORT_PATH, [])
-CALLBACK = lambda x: x
+NOOP_CALLBACK = lambda x: x
 
 DUMMY_MODULE_1_FILE = os.path.abspath(DUMMY_MODULE_1.__file__)
 DUMMY_MODULE_2_FILE = os.path.abspath(DUMMY_MODULE_2.__file__)
@@ -76,7 +98,7 @@ def setUp(self):
 
     @patch("streamlit.watcher.LocalSourcesWatcher.FileWatcher")
     def test_just_script(self, fob):
-        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, CALLBACK)
+        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, NOOP_CALLBACK)
 
         fob.assert_called_once()
         args = fob.call_args.args
@@ -94,7 +116,7 @@ def test_just_script(self, fob):
 
     @patch("streamlit.watcher.LocalSourcesWatcher.FileWatcher")
     def test_script_and_2_modules_at_once(self, fob):
-        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, CALLBACK)
+        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, NOOP_CALLBACK)
 
         fob.assert_called_once()
 
@@ -126,7 +148,7 @@ def test_script_and_2_modules_at_once(self, fob):
 
     @patch("streamlit.watcher.LocalSourcesWatcher.FileWatcher")
     def test_script_and_2_modules_in_series(self, fob):
-        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, CALLBACK)
+        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, NOOP_CALLBACK)
 
         fob.assert_called_once()
 
@@ -160,7 +182,7 @@ def test_script_and_2_modules_in_series(self, fob):
 
     @patch("streamlit.watcher.LocalSourcesWatcher.FileWatcher")
     def test_misbehaved_module(self, fob):
-        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, CALLBACK)
+        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, NOOP_CALLBACK)
 
         fob.assert_called_once()
 
@@ -171,14 +193,15 @@ def test_misbehaved_module(self, fob):
         fob.assert_called_once()  # Just __init__.py
 
     @patch("streamlit.watcher.LocalSourcesWatcher.FileWatcher")
-    def test_blacklist(self, fob):
+    def test_config_blacklist(self, fob):
+        """Test server.folderWatchBlacklist"""
         prev_blacklist = config.get_option("server.folderWatchBlacklist")
 
         config.set_option(
             "server.folderWatchBlacklist", [os.path.dirname(DUMMY_MODULE_1.__file__)]
         )
 
-        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, CALLBACK)
+        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, NOOP_CALLBACK)
 
         fob.assert_called_once()
 
@@ -192,6 +215,35 @@ def test_blacklist(self, fob):
         # Reset the config object.
         config.set_option("server.folderWatchBlacklist", prev_blacklist)
 
+    @patch("streamlit.watcher.LocalSourcesWatcher.FileWatcher")
+    def test_auto_blacklist(self, _):
+        prev_blacklist = config.get_option("server.folderWatchBlacklist")
+        config.set_option("server.folderWatchBlacklist", [])
+
+        lso = LocalSourcesWatcher.LocalSourcesWatcher(REPORT, NOOP_CALLBACK)
+
+        def is_blacklisted(filepath):
+            return any(
+                _file_is_in_folder(filepath, blacklisted_folder)
+                for blacklisted_folder in lso._folder_blacklist
+            )
+
+        # miniconda, anaconda, and .*/ folders should be blacklisted
+        self.assertTrue(is_blacklisted("/foo/miniconda2/script.py"))
+        self.assertTrue(is_blacklisted("/foo/miniconda3/script.py"))
+        self.assertTrue(is_blacklisted("/foo/anaconda2/script.py"))
+        self.assertTrue(is_blacklisted("/foo/anaconda3/script.py"))
+        self.assertTrue(is_blacklisted("/foo/.virtualenv/script.py"))
+        self.assertTrue(is_blacklisted("/foo/.venv/script.py"))
+        self.assertTrue(is_blacklisted("/foo/.random_hidden_folder/script.py"))
+
+        # Ensure we're not accidentally blacklisting things we shouldn't be
+        self.assertFalse(is_blacklisted("/foo/not_blacklisted/script.py"))
+        self.assertFalse(is_blacklisted("/foo/not_blacklisted/.hidden_script.py"))
+
+        # Reset the config object.
+        config.set_option("server.folderWatchBlacklist", prev_blacklist)
+
 
 def sort_args_list(args_list):
     return sorted(args_list, key=lambda args: args[0])


Code review comments:
diff_hunk: @@ -61,6 +66,11 @@ def __init__(self, report, on_file_changed):
 
         self._folder_blacklist = config.get_option("server.folderWatchBlacklist")
 
+        # Blacklist some additional folders, using glob syntax.
+        self._folder_blacklist.extend(
+            ["**/.*", "**/anaconda2", "**/anaconda3", "**/miniconda2", "**/miniconda3"]
path: lib/streamlit/watcher/LocalSourcesWatcher.py
text: @user1:
Move this list to a top-level constant, `DEFAULT_FOLDER_BLACKLIST`

@user1:
Ok, since we're releasing tomorrow I just fixed this :)