Repository: sqlfluff/sqlfluff
Pull request: Rename --parallel to --processes
Resolves #1057

Problem statement:
Better name for --parallel?
I feel that --threads instead of --parallel is clearer in terms of what the input value does. We can also then change the help string to specify that the number passed is the number of threads used.

Hints from issue discussion:
`--threads` was the original suggestion. I used `--parallel` for two main reasons:
* Generally, to avoid incorrect terminology: the implementation uses processes rather than threads. (Python threads do not scale well for this kind of processing due to the [GIL](https://wiki.python.org/moin/GlobalInterpreterLock).)
* The GNU `xargs` command uses [`-P` for parallel processing](https://man7.org/linux/man-pages/man1/xargs.1.html).

Is there another term we may want to consider? 
Maybe `--parallelism`? If it was just a boolean, `--parallel` makes sense, but as it's a configuration parameter less so (to me).
I'm open to change, but would like input from a few more people to try and have some consensus. @alanmcruickshank @dmateusp @nolanbconaway @pwildenhain 

Like? Dislike? Other suggestions?
`processes`
`processors`

Agree that `parallel` sounds like a Boolean.
I personally like `--processes`, but mostly because i have never successfully spelled parallel correctly on my first attempt!
I like that too!

On Sun, 23 May 2021, 20:30 Nolan Conaway, ***@***.***> wrote:

> I personally like --processes, but mostly because i have never
> successfully spelled parallel correctly on my first attempt!
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/sqlfluff/sqlfluff/issues/1057#issuecomment-846612732>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AFU7U4JGLY7MGXTDJWE326DTPFJUZANCNFSM446I7YAQ>
> .
>

Sounds good to me. Let's do this after @alanmcruickshank fixes the dbt parallelism issue. (Boo, merge conflicts!)

Commit message: Merge branch 'master' into nw/processes

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f25ad70e600..3c8bb88357a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 ### Changed
+- Renamed --parallel CLI argument to --processes to be more accurate.
 - L034 now ignores select statements which contain macros.
 - L034 now ignores select statements part of a set expression, most commonly a union.
 - Respect XDG base dirs on Mac OS ([#889](https://github.com/sqlfluff/sqlfluff/issues/889)).
@@ -18,8 +19,8 @@ Contributors:
 - [@bolajiwahab](https://github.com/bolajiwahab) ([#1063])(https://github.com/sqlfluff/sqlfluff/pull/1063)
 ## [0.6.1] - 2021-06-01
 ### Changed
-- Added support for additional delimiters by creating a new DelimiterSegment in the 
-  ANSI dialect which defaults to the semicolon, but allows it to be more intuitive 
+- Added support for additional delimiters by creating a new DelimiterSegment in the
+  ANSI dialect which defaults to the semicolon, but allows it to be more intuitive
   when overriden in a specific child dialect (mysql) [#901](https://github.com/sqlfluff/sqlfluff/issues/901))
 - Added support for the DELIMITER statement in the mysql dialect [#901](https://github.com/sqlfluff/sqlfluff/issues/901))
 ## [0.6.0a2] - 2021-05-27
diff --git a/src/sqlfluff/cli/commands.py b/src/sqlfluff/cli/commands.py
index e7c8701be44..b64caafa963 100644
--- a/src/sqlfluff/cli/commands.py
+++ b/src/sqlfluff/cli/commands.py
@@ -288,7 +288,7 @@ def dialects(**kwargs):
 )
 @click.option(
     "-p",
-    "--parallel",
+    "--processes",
     type=int,
     default=1,
     help="If set to a value higher than 1, run SQLFluff in parallel, "
@@ -297,7 +297,7 @@ def dialects(**kwargs):
 @click.argument("paths", nargs=-1)
 def lint(
     paths,
-    parallel,
+    processes,
     format,
     nofail,
     disregard_sqlfluffignores,
@@ -344,7 +344,7 @@ def lint(
                 paths,
                 ignore_non_existent_files=False,
                 ignore_files=not disregard_sqlfluffignores,
-                parallel=parallel,
+                processes=processes,
             )
         except OSError:
             click.echo(
@@ -410,14 +410,14 @@ def do_fixes(lnt, result, formatter=None, **kwargs):
     "--fixed-suffix", default=None, help="An optional suffix to add to fixed files."
 )
 @click.option(
-    "--parallel",
+    "--processes",
     type=int,
     default=1,
     help="If set to a value higher than 1, run SQLFluff in parallel, "
     "speeding up processing.",
 )
 @click.argument("paths", nargs=-1)
-def fix(force, paths, parallel, bench=False, fixed_suffix="", logger=None, **kwargs):
+def fix(force, paths, processes, bench=False, fixed_suffix="", logger=None, **kwargs):
     """Fix SQL files.
 
     PATH is the path to a sql file or directory to lint. This can be either a
@@ -449,7 +449,7 @@ def fix(force, paths, parallel, bench=False, fixed_suffix="", logger=None, **kwa
     click.echo("==== finding fixable violations ====")
     try:
         result = lnt.lint_paths(
-            paths, fix=True, ignore_non_existent_files=False, parallel=parallel
+            paths, fix=True, ignore_non_existent_files=False, processes=processes
         )
     except OSError:
         click.echo(
diff --git a/src/sqlfluff/core/__init__.py b/src/sqlfluff/core/__init__.py
index e6d23b30abb..2e56f33548a 100644
--- a/src/sqlfluff/core/__init__.py
+++ b/src/sqlfluff/core/__init__.py
@@ -26,7 +26,7 @@
 from sqlfluff.core.timing import TimingSummary
 
 
-# This is for "sqlfluff lint" and "sqlfluff fix" multiprocessing (--parallel)
+# This is for "sqlfluff lint" and "sqlfluff fix" multiprocessing (--processes)
 # support. If an exception (i.e. runtime error) occurs in a worker process, we
 # want to return the tracebook to the main process and report it there, as part
 # of the normal output. However, anything returned from a multiprocessing.Pool
diff --git a/src/sqlfluff/core/linter/linter.py b/src/sqlfluff/core/linter/linter.py
index ee51c877972..9e7566f303d 100644
--- a/src/sqlfluff/core/linter/linter.py
+++ b/src/sqlfluff/core/linter/linter.py
@@ -763,7 +763,7 @@ def lint_path(
         fix: bool = False,
         ignore_non_existent_files: bool = False,
         ignore_files: bool = True,
-        parallel: int = 1,
+        processes: int = 1,
     ) -> LintedDir:
         """Lint a path."""
         linted_path = LintedDir(path)
@@ -779,7 +779,7 @@ def lint_path(
         runner = get_runner(
             self,
             self.config,
-            parallel=parallel,
+            processes=processes,
             allow_process_parallelism=self.allow_process_parallelism,
         )
         for linted_file in runner.run(fnames, fix):
@@ -796,7 +796,7 @@ def lint_paths(
         fix: bool = False,
         ignore_non_existent_files: bool = False,
         ignore_files: bool = True,
-        parallel: int = 1,
+        processes: int = 1,
     ) -> LintingResult:
         """Lint an iterable of paths."""
         # If no paths specified - assume local
@@ -813,7 +813,7 @@ def lint_paths(
                     fix=fix,
                     ignore_non_existent_files=ignore_non_existent_files,
                     ignore_files=ignore_files,
-                    parallel=parallel,
+                    processes=processes,
                 )
             )
         result.stop_timer()
diff --git a/src/sqlfluff/core/linter/runner.py b/src/sqlfluff/core/linter/runner.py
index 0870596673e..b72151daa0d 100644
--- a/src/sqlfluff/core/linter/runner.py
+++ b/src/sqlfluff/core/linter/runner.py
@@ -105,9 +105,9 @@ class ParallelRunner(BaseRunner):
     # don't pickle well.
     pass_formatter = False
 
-    def __init__(self, linter, config, parallel):
+    def __init__(self, linter, config, processes):
         super().__init__(linter, config)
-        self.parallel = parallel
+        self.processes = processes
 
     def run(self, fnames, fix):
         """Parallel implementation.
@@ -118,7 +118,7 @@ def run(self, fnames, fix):
         and linting work out to the threads.
         """
         with self._create_pool(
-            self.parallel,
+            self.processes,
             self._init_global,
             (self.config,),
         ) as pool:
@@ -212,22 +212,22 @@ def reraise(self):
 def get_runner(
     linter,
     config,
-    parallel: int,
+    processes: int,
     allow_process_parallelism: bool = True,
 ) -> BaseRunner:
     """Generate a runner instance based on parallel and sytem configuration."""
     # Python multiprocessing isn't supported in 3.6 and before.
     # The library exists but we get pickling errors with LintedFile.
-    if parallel > 1 and sys.version_info >= (3, 7):
+    if processes > 1 and sys.version_info >= (3, 7):
         # Process parallelism isn't really supported during testing
         # so this flag allows us to fall back to a threaded runner
         # in those cases.
         if allow_process_parallelism:
-            return MultiProcessRunner(linter, config, parallel=parallel)
+            return MultiProcessRunner(linter, config, processes=processes)
         else:
-            return MultiThreadRunner(linter, config, parallel=parallel)
+            return MultiThreadRunner(linter, config, processes=processes)
     else:
-        if parallel > 1:
+        if processes > 1:
             linter_logger.warning(
                 "Parallel linting is not supported in Python %s.%s.",
                 sys.version_info.major,
diff --git a/test/core/linter_test.py b/test/core/linter_test.py
index 1eac62a5e28..d71ba23f7ea 100644
--- a/test/core/linter_test.py
+++ b/test/core/linter_test.py
@@ -186,8 +186,8 @@ def test__linter__linting_result_check_tuples_by_path(by_path, result_type):
     isinstance(check_tuples, result_type)
 
 
-@pytest.mark.parametrize("parallel", [1, 2])
-def test__linter__linting_result_get_violations(parallel):
+@pytest.mark.parametrize("processes", [1, 2])
+def test__linter__linting_result_get_violations(processes):
     """Test that we can get violations from a LintingResult."""
     lntr = Linter()
     result = lntr.lint_paths(
@@ -195,7 +195,7 @@ def test__linter__linting_result_get_violations(parallel):
             "test/fixtures/linter/comma_errors.sql",
             "test/fixtures/linter/whitespace_errors.sql",
         ],
-        parallel=parallel,
+        processes=processes,
     )
 
     all([type(v) == SQLLintError for v in result.get_violations()])
@@ -234,7 +234,7 @@ def imap_unordered(self, *args, **kwargs):
     lntr = Linter(formatter=CallbackFormatter(callback=lambda m: None, verbosity=0))
     result = lntr.lint_paths(
         ("test/fixtures/linter/comma_errors.sql",),
-        parallel=2,
+        processes=2,
     )
 
     all([type(v) == SQLLintError for v in result.get_violations()])
@@ -247,7 +247,7 @@ def test_lint_path_parallel_wrapper_exception(patched_lint):
     Test on MultiThread runner because otherwise we have pickling issues.
     """
     patched_lint.side_effect = ValueError("Something unexpected happened")
-    for result in runner.MultiThreadRunner(Linter(), FluffConfig(), parallel=1).run(
+    for result in runner.MultiThreadRunner(Linter(), FluffConfig(), processes=1).run(
         ["test/fixtures/linter/passing.sql"], fix=False
     ):
         assert isinstance(result, runner.DelayedException)


Code review comments:
diff_hunk: @@ -288,7 +288,7 @@ def dialects(**kwargs):
 )
 @click.option(
     "-p",
-    "--parallel",
+    "--processes",
     type=int,
     default=1,
     help="If set to a value higher than 1, run SQLFluff in parallel, "
path: src/sqlfluff/cli/commands.py
text: @user1:
```suggestion
    help="If set to a value higher than 1, starts multiple SQLFluff processes, "
```