Simple API config path

[jpy-git]

Brief summary of the change made

Fixes #1274. Builds upon my CLI config path additions #1986 and #2061.
Adds an option to pass a config_path to the Simple API. This doesn't have the same override behaviour as the CLI as it doesn't make sense in the context of the API. The config_path specified is the config that is used.

Also changes the API to create a config object rather than passing its args directly to the linter. This should help decouple the linter and the API and also make it easy to update the simple API with additional arguments from the CLI in the future.

Are there any other side effects of this change that we should be aware of?

No

Pull Request checklist

• Please confirm you have completed any of the necessary steps below.

• Included test cases to demonstrate any code changes, which may be one or more of the following:

  • .yml rule test cases in test/fixtures/rules/std_rule_cases.
  • .sql/.yml parser test cases in test/fixtures/dialects (note YML files can be auto generated with python test/generate_parse_fixture_yml.py or by running tox locally).
  • Full autofix test cases in test/fixtures/linter/autofix.
  • Other.

• Added appropriate documentation for the change.

• Created GitHub issues for any relevant followup/future enhancements if appropriate.

[jpy-git]

we can hopefully remove those dialect/rules/exclude_rules arguments to the linter in the future as they should be part of the config. i.e. decouple linter and simple API 👍

[tunetheweb]

That would mean you'd have to have a config file to pass. Might be overkill for some simple options. For example I presume https://online.sqlfluff.com/ uses the simple API and just needs to specify the dialect.

But it is duplication which is annoying.

BTW what happens if you specify a dialect AND a config file? Which ones wins out? Or should that error and it should be either/or but not both?

[jpy-git]

@tunetheweb you can still pass a dialect to the Simple API (and the dialect would overwrite that in the config as you'd expect)?
The Simple API itself is unchanged from a user perspective:

res = sqlfluff.lint(
    dialect="snowflake",
    rules=["L001", "L002"],
    config_path="some/path/to/config/.sqlfluff"
)

I'm specifically talking about Linter in the comment which doesn't need those arguments 😄

[jpy-git]

as in we don't need to be able to do Linter(dialect="snowflake") since the simple api was the only thing using that. Everything else does Linter(config=cfg).

[tunetheweb]

Ah Gothca.

Yes In agree explicit dialect or rule should override config, so that's good, but I wonder if we need to expose this in the docs though so others know that's the way it's implemented?

[jpy-git]

@barrywhart @tunetheweb config_path option for the Simple API 😄

[codecov]

Codecov Report

Merging #2080 (166d2b3) into main (d8adba3) will decrease coverage by 0.03%.
The diff coverage is 87.50%.

❗ Current head 166d2b3 differs from pull request most recent head e258668. Consider uploading reports for the commit e258668 to get more accurate results

@@             Coverage Diff             @@
##              main    #2080      +/-   ##
===========================================
- Coverage   100.00%   99.96%   -0.04%     
===========================================
  Files          148      148              
  Lines        10538    10548      +10     
===========================================
+ Hits         10538    10544       +6     
- Misses           0        4       +4     

Impacted Files	Coverage Δ
src/sqlfluff/api/simple.py	94.59% <87.50%> (-5.41%)	⬇️
src/sqlfluff/core/config.py	99.35% <0.00%> (-0.65%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8adba3...e258668. Read the comment docs.

[tunetheweb]

Any idea what's up with docs as the parameter name doesn't appear (and also not for exclude_rules):

It appears fine on production for exclude_rules:

[jpy-git]

@tunetheweb Interesting let me look at that. Also need to see where I'm missing coverage.

[jpy-git]

@tunetheweb updated the docstrings if you want to take another look?

Still working on coverage in the meantime

[tunetheweb]

Docs look better now.

[tunetheweb]

Found 4 of your lines.

The other two lines are due to this in config.py:

Guess the simple api tests were covering that before, but not you instantiate a config object with those overrides so no longer directly call this.

So guess you need to either remove that functionality completely or replace that test coverage with complex API tests?

[tunetheweb]

Good work! We’ve needed this for some time.