Add sql backend + sqlfluff linter

[grihabor]

No description provided.

[kaos]

lgtm bar some nits.

Also miss some file headers, which I think will be added if you run pants fmt (or maybe pants fix?) by the preamble backend.

[grihabor]

Thanks for an early review!

Fixed the issues

[grihabor]

How do I make sure the tests only run for python>=3.8?

[kaos]

How do I make sure the tests only run for python>=3.8?

Look at how these have been applied:
https://github.com/pantsbuild/pants/blob/main/src/python/pants/testutil/python_interpreter_selection.py

[cburroughs]

oh, for Sqlfluff, yeah! 🎉

[kaos]

Would be great with a few more lines of docs, describing at least briefly how to enable and use this backend :)
(I think that there are two backends to enable could trip at least new users up..)

[grihabor]

@kaos Sure, added the docs

[kaos]

Thanks!

[benjyw]

Thanks for this @grihabor, exciting to have a new useful backend like this.

I do have one comment which may not be immediately actionable: I wonder if we actually need targets at all for this sort of backend? It seems like we could just treat all *.sql files appropriately and not have explicit targets (even tailored ones) at all.

That said, it would be the first time we'd done something like that, and there may be gotchas, so I don't expect you to make that change. But as I think we should be moving towards that sort of paradigm in the future, this backend might be a good place to experiment with it when we start looking into it.

[grihabor]

Thank you for review!

I wonder if we actually need targets at all for this sort of backend?

I don't know, I just did it similar to other backends. Right now we're using this as a plugin and the .sql is actually a python template string which we render in the application. I'm thinking about moving sql rendering directly into pants in the future, so that the application consumes ready to go sql without templating, so I will probably need some codegen with jinja for sql, but I'm not sure it's a good idea yet.

[kaos]

Thanks for this @grihabor, exciting to have a new useful backend like this.

I do have one comment which may not be immediately actionable: I wonder if we actually need targets at all for this sort of backend? It seems like we could just treat all *.sql files appropriately and not have explicit targets (even tailored ones) at all.

That said, it would be the first time we'd done something like that, and there may be gotchas, so I don't expect you to make that change. But as I think we should be moving towards that sort of paradigm in the future, this backend might be a good place to experiment with it when we start looking into it.

Ah, right. This is an excellent comment, and also not accurate, as we do have the yamllint backend which does just that (being a target-less linter)

Sorry for not running on all cylinders here.. failing to make this connection myself.

[kaos]

yamllint was added in #17821

[kaos]

to be clear, I'm fine keeping this as-is for now as well.. just good to be aware of the alternative to consider going forward :)

[huonw]

Thanks for the contribution.

When you have a chance, please merge main (or rebase onto it) and add some release notes to docs/notes/2.22.x.md (maybe in a #### NEW: SQL section). See #20888 for more info.

[grihabor]

add some release notes

@huonw Here you go bb28732

[kaos]

Looks good, thanks!

Let's get this in so we can start iterate on it, if needed :)

[ndellosa95]

Looks good, thanks!

Let's get this in so we can start iterate on it, if needed :)

Just saw this and did a double take because I also have a sqlfluff backend in an in-repo plugin designed to work with dbt. We can definitely reconcile what's here and what we have though.