Click errors for SQLFluff

[tunetheweb]

Describe the bug A clear and concise description of what the bug is.

Using SQLFLuff is producing errors like this:

--Error detail:
Traceback (most recent call last):
  File "/usr/local/bin/sqlfluff", line 5, in <module>
    from sqlfluff.cli.commands import cli
  File "/usr/local/lib/python3.9/site-packages/sqlfluff/cli/commands.py", line 28, in <module>
    from sqlfluff.cli.autocomplete import dialect_shell_complete
  File "/usr/local/lib/python3.9/site-packages/sqlfluff/cli/autocomplete.py", line 3, in <module>
    from click.shell_completion import CompletionItem
ModuleNotFoundError: No module named 'click.shell_completion'

To Reproduce Steps to reproduce the behavior:

Add a workflow like this:

    steps:
      - name: Checkout code
        uses: actions/checkout@v2

      - name: sqlfluff_mysql
        uses: megalinter/megalinter/flavors/python@v5
        env:
          DEFAULT_BRANCH: master
          GITHUB_TOKEN: ${{ secrets.GITHUB_DEPLOYMENT_TOKEN }}
          GITHUB_COMMENT_REPORTER: false
          PRINT_ALPACA: false
          VALIDATE_SQL_SQLFLUFF: true
          SQL_SQLFLUFF_DISABLE_ERRORS: true
          SQL_SQLFLUFF_ARGUMENTS: --dialect=mysql
          FILTER_REGEX_INCLUDE: (dir1\/dir2\/dir3\/.*\.sql)

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context This is due to a bug upstream in SQLFluff in 0.9.3. We added a function only available in click 8.0 and above, but only updated our requirements.txt (used for local installs) and not our setup.cfg (used for pip install).

We're releasing 0.9.4 now to fix it.

[tunetheweb]

@nvuillam any ideas why the install doesn't use the latest click? We used to require click>=7.1 but not required click>=8.0 but unless there's a reason for the lower version shouldn't it always take the latest?

[nvuillam]

@tunetheweb there is a pip install for all python packages

I'm no pip expert, but probably another python package has a constraint on a lower version of click, compliant with sqlfluff dependency requirement defined in setup.cfg

[tunetheweb]

Yeah that's what worries me. So if our latest release demands a higher version will it not work? Or do you have a process to handle this in a separate env?

[nvuillam]

Yeah that's what worries me. So if our latest release demands a higher version will it not work? Or do you have a process to handle this in a separate env?

Can't promise, but if it works with all the other packages, I'm pretty confident that it will work with sqlfluff :) I could work with venv but i'm afraid that if i create a venv for each linter, it will dramatically increase the docker image size and i'd like to avoid that :/

[tunetheweb]

SQLFluff 0.9.4 is released there if you wanna try it out?

And if you could include in your next release that would be much appreciated!

[nvuillam]

Auto-update job runs every night but for the occasion I just launched it, result in one big hour :) ( it will detect a new version and create a PR that will trigger test classes )

https://github.com/megalinter/megalinter/runs/4998717020?check_suite_focus=true

[tunetheweb]

Fingers cross we can get a release out and stop breaking you for once! Apologies for the errors in 0.9.2 and 0.9.3!

[nvuillam]

When did you release the version that provokes the crash ?

5 hours ago, SqlFluff test cases worked perfectly fine -> https://github.com/megalinter/megalinter/runs/4997390251?check_suite_focus=true

But maybe the sample file do not go thru the crashing code :/

[tunetheweb]

@tdstark raised it to us after your last release, and it was an error I recognised whereby we discovered our issue.

Not sure why your test suite didn't get it to be honest. That presumably runs in the combined Docker env so should be the same as what was released?

[nvuillam]

I don't have an explanation :/

[nvuillam]

@tunetheweb would you have some more extended working sql test files so I can replace the dramatically simple one that is used in MegaLinter tests ? :D

[tunetheweb]

Yeah let me try and reproduce with your sample SQLs. Would be good to understand this.

[tunetheweb]

I can reproduce it with that, and an old version of click:

% pip install click==7.1.2
Collecting click==7.1.2
  Downloading click-7.1.2-py2.py3-none-any.whl (82 kB)
     |████████████████████████████████| 82 kB 1.0 MB/s            
Installing collected packages: click
  Attempting uninstall: click
    Found existing installation: click 8.0.3
    Uninstalling click-8.0.3:
      Successfully uninstalled click-8.0.3
ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
black 22.1.0 requires click>=8.0.0, but you have click 7.1.2 which is incompatible.
Successfully installed click-7.1.2

% sqlfluff lint test.sql
Traceback (most recent call last):
  File "/Users/barry/sqlfluff/.venv/bin/sqlfluff", line 33, in <module>
    sys.exit(load_entry_point('sqlfluff', 'console_scripts', 'sqlfluff')())
  File "/Users/barry/sqlfluff/.venv/bin/sqlfluff", line 25, in importlib_load_entry_point
    return next(matches).load()
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/lib/python3.8/importlib/metadata.py", line 77, in load
    module = import_module(match.group('module'))
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/Users/barry/sqlfluff/src/sqlfluff/cli/commands.py", line 28, in <module>
    from sqlfluff.cli.autocomplete import dialect_shell_complete
  File "/Users/barry/sqlfluff/src/sqlfluff/cli/autocomplete.py", line 3, in <module>
    from click.shell_completion import CompletionItem
ModuleNotFoundError: No module named 'click.shell_completion'

Where test.sql contains delete from person where 1=1;.

Same with the following:

% echo "delete from person where 1=1;" | sqlfluff lint -
Traceback (most recent call last):
  File "/Users/barry/sqlfluff/.venv/bin/sqlfluff", line 33, in <module>
    sys.exit(load_entry_point('sqlfluff', 'console_scripts', 'sqlfluff')())
  File "/Users/barry/sqlfluff/.venv/bin/sqlfluff", line 25, in importlib_load_entry_point
    return next(matches).load()
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/lib/python3.8/importlib/metadata.py", line 77, in load
    module = import_module(match.group('module'))
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/Users/barry/sqlfluff/src/sqlfluff/cli/commands.py", line 28, in <module>
    from sqlfluff.cli.autocomplete import dialect_shell_complete
  File "/Users/barry/sqlfluff/src/sqlfluff/cli/autocomplete.py", line 3, in <module>
    from click.shell_completion import CompletionItem
ModuleNotFoundError: No module named 'click.shell_completion'

pip install click so it uses a later version avoids the error.

Are your tests run in isolation (in which case SQLFluff might have installed the latest as we only had a minimum and not a maximum version requirement) or are they run on the candidate combined Docker file (which may have used a different version if someone other package has a max dependency on click)?

[tunetheweb]

Looks like it was snakefmt that requires the lower version btw:

snakefmt 0.4.4 requires click<8.0.0,>=7.1.2, but you have click 8.0.3 which is incompatible.

That was downgraded yesterday: https://github.com/megalinter/megalinter/pull/1218.

Could it be SQLFluff tests passed before this, then that downgrade happened, and hence the error? Or should all the tests be carried out on the final build?

[nvuillam]

snakemake snakefmt generated a big mess between my latest tests of the beta and v5.7.0 releases.. I didn't have the choice but to downgrade them :/

It will probably be solved in a latest snakefmt release

[tunetheweb]

So that’s likely the reason your SQLFluff tests didn’t fail? Because of manual interventation? And normally it would have caught this? If so then at least we have an explanation.

And I can hardly complain about last minute downgrades since I recently made you do one for SQLFluff! 😁

[nvuillam]

In CI , sqlfluff didn't fail with v0.9.3 and click v7.1.2 :)

Isn't it the case it's supposed to fail ? 🤣

[tunetheweb]

Yes. Thats why I’m confused. I just repeated running sqlfluff with click v7.1.2 and your test file and I get the fail. So why didn’t you?

[nvuillam]

You're using python 3.8 in mac , MegaLinter is based on python:3.9.7-alpine3.13 on alpine linux ? :/

But that shouldn't make a difference :/

About your test of MegaLinter, you also add --dialect=mysql , which I do not in MegaLinter test classes

[tunetheweb]

You're using python 3.8 in mac , MegaLinter is based on python:3.9.7-alpine3.13 on alpine linux ? :/

But that shouldn't make a difference :/

Yeah that shouldn’t matter, especially as error was seen with latest Mega Linter release.

About your test of MegaLinter, you also add --dialect=mysql , which I do not in MegaLinter test classes

That doesn’t matter. The delete statement is valid SQL in both ANSI (the default) and MySQL. My test above was completed with default ANSI without that param.

[tunetheweb]

When I look at the logs of the action: https://pipelines.actions.githubusercontent.com/CZNV7JsLKXAlORNSVk143wMHBd79LmRg3nz53BwEDVVX6AOjqA/_apis/pipelines/1/runs/52733/signedlogcontent/2?urlExpires=2022-01-30T22%3A02%3A47.1399361Z&urlSigningMethod=HMACV1&urlSignature=PpxcCawXOA%2BcYpKtOyPybtmF0wXRlBXazMVW9fBzFR0%3D

it does look like it downloads different versions of click:

…
2022-01-30T03:21:45.1449647Z 03:21:45| Downloading click-7.1.2-py2.py3-none-any.whl (82 kB)
…
2022-01-30T03:45:10.1198087Z 03:45:10| Downloading click-8.0.3-py3-none-any.whl (97 kB)
…
2022-01-30T03:51:51.3699886Z   Downloading click-8.0.3-py3-none-any.whl (97 kB)
…

And then the SQLFluff tests were completed after that (so presumably on that final install of 8.0.3?):

…
2022-01-30T03:52:42.6185387Z 2022-01-30 03:52:42,608 [INFO] Updated /tmp/lint/.automation/../megalinter/tests/test_megalinter/linters/sql_sqlfluff_test.py
…

That could be why it didn’t fail? Then the Image was built with 7.1.2 and that would make it fail.

[tdstark]

FWIW I'm still getting click errors after the update.

[nvuillam]

@tdstark do you have an example SQL file that triggers the error ?

[tunetheweb]

But no update has been released to include sqlfluff 0.9.4, so not surprising you're still getting the error.

The only mystery is why the tests didn't catch it but, as per my comment above, it looks to me like the tests may not be set up to catch this. Perhaps they should be changed to also run on the final Docker image as well?

[tdstark]

@nvuillam it doesn't appear to even hit the sql. I get the following error and then the sql is skipped:

  from sqlfluff.cli.commands import cli
File "/usr/local/lib/python3.9/site-packages/sqlfluff/cli/commands.py", line 28, in <module>
  from sqlfluff.cli.autocomplete import dialect_shell_complete
File "/usr/local/lib/python3.9/site-packages/sqlfluff/cli/autocomplete.py", line 3, in <module>
  from click.shell_completion import CompletionItem
ModuleNotFoundError: No module named 'click.shell_completion'

Here is my SQLFluff config in Github Actions:

      - name: sqlfluff_mysql
        uses: megalinter/megalinter/flavors/python@v5
        env:
          DEFAULT_BRANCH: master
          GITHUB_TOKEN: ${{ secrets.CII_GITHUB_DEPLOYMENT_TOKEN }}
          GITHUB_COMMENT_REPORTER: false
          PRINT_ALPACA: false
          VALIDATE_SQL_SQLFLUFF: true
          SQL_SQLFLUFF_ARGUMENTS: --dialect=mysql
          FILTER_REGEX_INCLUDE: (dir1\/dir2\/dir3\/.*\.sql)

[tdstark]

But no update has been released to include sqlfluff 0.9.4, so not surprising you're still getting the error.

Oh my mistake. I had it in my head for a moment that an update was pushed.

[nvuillam]

@tunetheweb the tests are performed on the final docker image that has just been tested

There has been numerous auto-updates for beta version, and none of them installed a sqlfluff version higher than 0.9.3 I don't understand, because Pipy indicates Requires: Python >=3.7 and MegaLinter is based on python:3.9.7-alpine3.13 Maybe there is some dependency with a dependent package... like click ? (edit: not click -> Requires: Python >=3.6 )

I have the same issue with black python formatter, that does not retrieves the latest version

[tunetheweb]

@tunetheweb the tests are performed on the final docker image that has just been tested

Then I'm so confused as to how that passed. Cause SQLFluff 0.9.3 is busted on old click, and looks like the Docker image uses old click :-(

There has been numerous auto-updates for beta version, and none of them installed a sqlfluff version higher than 0.9.3

I'm guessing it can't install it, cause of a dependency clash (e.g. we require a newer click (to resolve this issue), and another package required an older click). Where are the log files including the pip install?

[nvuillam]

From a build of today:

Step 27/109 : RUN pip3 install --no-cache-dir --upgrade           'cpplint'           'cfn-lint'           'pylint'           'black'           'flake8'           'isort'           'bandit'           'mypy'           'restructuredtext_lint'           'rstcheck'           'sphinx<4.0'           'rstfmt'           'snakemake==6.13.1'           'snakefmt==0.4.4'           'sqlfluff'           'yamllint'

....

Collecting sqlfluff
  Downloading sqlfluff-0.9.4-py3-none-any.whl (449 kB)

.... and later in the same command log

Collecting sqlfluff
  Downloading sqlfluff-0.9.3-py3-none-any.whl (447 kB)

... and finally, 0.9.3 is installed !

Successfully installed Jinja2-3.0.3 MarkupSafe-2.0.1 Pygments-2.11.2 alabaster-0.7.12 appdirs-1.4.4 astroid-2.9.3 attrs-21.4.0 aws-sam-translator-1.42.0 babel-2.9.1 bandit-1.7.2 black-21.12b0 boto3-1.20.46 botocore-1.23.46 certifi-2021.10.8 cfn-lint-0.58.0 chardet-4.0.0 charset-normalizer-2.0.11 click-7.1.2 colorama-0.4.4 configargparse-1.5.3 connection-pool-0.0.3 cpplint-1.5.5 datrie-0.8.2 diff-cover-6.4.4 docutils-0.16 filelock-3.4.2 flake8-4.0.1 gitdb-4.0.9 gitpython-3.1.26 idna-3.3 imagesize-1.3.0 importlib-metadata-1.7.0 iniconfig-1.1.1 ipython-genutils-0.2.0 isort-5.10.1 jmespath-0.10.0 jschema-to-python-1.2.3 jsonpatch-1.32 jsonpickle-2.1.0 jsonpointer-2.2 jsonschema-3.2.0 junit-xml-1.9 jupyter-core-4.9.1 lazy-object-proxy-1.7.1 mccabe-0.6.1 mypy-0.931 mypy-extensions-0.4.3 nbformat-5.1.3 networkx-2.6.3 packaging-21.3 pathspec-0.9.0 pbr-5.8.0 platformdirs-2.4.1 pluggy-1.0.0 psutil-5.9.0 pulp-2.6.0 py-1.11.0 pycodestyle-2.8.0 pyflakes-2.4.0 pylint-2.12.2 pyparsing-3.0.7 pyrsistent-0.18.1 pytest-6.2.5 python-dateutil-2.8.2 pytz-2021.3 pyyaml-6.0 ratelimiter-1.2.0.post0 regex-2022.1.18 requests-2.27.1 restructuredtext-lint-1.3.2 rstcheck-3.3.1 rstfmt-0.0.10 s3transfer-0.5.0 sarif-om-1.0.4 six-1.16.0 smart-open-5.2.1 smmap-5.0.0 snakefmt-0.4.4 snakemake-6.13.1 snowballstemmer-2.2.0 sphinx-3.5.4 sphinxcontrib-applehelp-1.0.2 sphinxcontrib-devhelp-1.0.2 sphinxcontrib-htmlhelp-2.0.0 sphinxcontrib-jsmath-1.0.1 sphinxcontrib-qthelp-1.0.3 sphinxcontrib-serializinghtml-1.1.5 sqlfluff-0.9.3 stevedore-3.5.0 stopit-1.1.2 tabulate-0.8.9 tblib-1.7.0 toml-0.10.2 tomli-1.2.3 toposort-1.7 tqdm-4.62.3 traitlets-5.1.1 typing-extensions-4.0.1 urllib3-1.26.8 wrapt-1.13.3 yamllint-1.26.3 zipp-3.7.0

It seems something manages to get the lower version after having found the latest one... My python expertise is not high enough to understand what happens, any good soul who can is very welcome !

[nvuillam]

Arghhhhh I think I have just been welcome in dependency hell >< ><

https://pip.pypa.io/en/stable/topics/dependency-resolution/#backtracking

[tunetheweb]

Yup. You are basically running this:

RUN pip3 install --no-cache-dir --upgrade \
          'cpplint' \
          'cfn-lint' \
          'pylint' \
          'black' \
          'flake8' \
          'isort' \
          'bandit' \
          'mypy' \
          'restructuredtext_lint' \
          'rstcheck' \
          'sphinx<4.0' \
          'rstfmt' \
          'snakemake==6.13.1' \
          'snakefmt==0.4.4' \
          'sqlfluff' \
          'yamllint'

That is you don't specify a sqlfluff version - so it will get the latest it can.

However do lock other packages, including snakefmt. So if it has packages that aren't compatible with sqlfluff 0.9.4 then it'll try older versions (as we haven't set a minimum version for sqlfluff).

This can be shown with the following by locking sqlfluff to 0.9.4 (and only installing snakefmt and sqlfluff):

% pip3 install --no-cache-dir --upgrade \
          'snakefmt==0.4.4' \
          'sqlfluff==0.9.4'

And you see this error:

ERROR: Cannot install snakefmt==0.4.4 and sqlfluff==0.9.4 because these package versions have conflicting dependencies.

The conflict is caused by:
    snakefmt 0.4.4 depends on click<8.0.0 and >=7.1.2
    sqlfluff 0.9.4 depends on click>=8.0

To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip attempt to solve the dependency conflict

ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/user_guide/#fixing-conflicting-dependencies
WARNING: You are using pip version 21.3.1; however, version 22.0.2 is available.
You should consider upgrading via the '/Users/barry/sources/test/.venv/bin/python -m pip install --upgrade pip' command.

So, unless you have separate venvs you're gonna have to pick for now :-(

Which is annoying as guess we both have breaking bugs...

[tunetheweb]

The following will work to exclude the bad sqlfluff versions btw:

          'sqlfluff!=0.9.1,!=0.9.2,!=0.9.3'

But it does mean you're all the way back to 0.9.0 for now (until you remove that snakefmt pinning). Still it's better than a broken 0.9.3!

[tunetheweb]

There's a lot to be said for node_modules that can handle this better rather than only having one shared space for dependencies!

[nvuillam]

grmblmblmblmblm black and sqlfluff are much more used than snakemake... but I can not add a breaking change in a minor version, and v6 won't be ready yet for weeks.....

node_modules handle it better.... but generates 500 megas of dependencies for each package :D

I'll ask snakefmt if they can remove the click<8.0.0 in their dependencies.... and meanwhile probably add the capability to temporary disable a linter :/

[tunetheweb]

grmblmblmblmblm black and sqlfluff are much more used than snakemake... but I can not add a breaking change in a minor version, and v6 won't be ready yet for weeks.....

For now just add this:

'sqlfluff!=0.9.1,!=0.9.2,!=0.9.3'

It'll install 0.9.0 (which works). Then it'll automatically upgrade to 0.9.4 when the dependency hell is broken.

node_modules handle it better.... but generates 500 megas of dependencies for each package :D

True dat! And now you know why as each module takes it's own copy :-)

I'll ask snakefmt if they can remove the click<8.0.0 in their dependencies.... and meanwhile probably add the capability to temporary disable a linter :/

Looks like they insist on 7.1.2: https://github.com/snakemake/snakefmt/blob/master/pyproject.toml#L18 Worth raising with them to see if they can unlock that.

[tunetheweb]

But after all this, I'm still confused as to why the tests passed! 0.9.3 is fundamentally broken if installed with an old click, so either:

1. the tests ran with the new click (but the final build did not) or
2. the tests are broken and not alerting when they fail

[tdstark]

I mean, FWIW I'd love the newest version of SQLFluff for all the improved dialect changes but that's just me.

[nvuillam]

@tunetheweb I asked them, let's wait & see... :) https://github.com/snakemake/snakefmt/issues/132

@tdstark you'll have it, and black is also widely used, and its latest version has been requested too :=)

[nvuillam]

Smells good :)

https://github.com/megalinter/megalinter/runs/5029301733?check_suite_focus=true

I have to sleep, but except if bad luck, new beta version with latest sqlfluff and black will be available tomorrow :)

[nvuillam]

And as a good news never comes alone, snakefmt maintainer is currently upgrading dependency :)

https://github.com/snakemake/snakefmt/issues/132#issuecomment-1027387365

[tunetheweb]

Thanks for merging that fix.

FYI we might be removing this strict requirement to allow us to play nicer with other (older) software: https://github.com/sqlfluff/sqlfluff/pull/2547

[tdstark]

Thanks for merging this! I tested the production branch is still broken but the beta branch appears to be operating as expected.

[nvuillam]

Thanks for merging this! I tested the production branch is still broken but the beta branch appears to be operating as expected.

I cleverly forgot to validate to release CI after creating 5.7.1... it will be ready in half an hour :)

[tunetheweb]

FYI SQLFluff 0.10.0 has just been released with less strict click requirements. So it should play nicer with other packages now (even really old ones that should know better! 😉).

[nvuillam]

Great, thanks for the feedback :)