sqlfluff / sqlfluff-github-actions

The official resource for SQLFluff related GitHub Actions
MIT License
100 stars 37 forks source link

SyntaxError: Unexpected token : in JSON at position 2 #15

Closed tlfbrito closed 2 years ago

tlfbrito commented 2 years ago

After updating to sqlfluff 0.9.0 the yuzutech/annotations-action started to fail with Error: SyntaxError: Unexpected token : in JSON at position 2 This even happens when the annotation contains an empty json ([]).

tunetheweb commented 2 years ago

Sorry but what has SQLFluff got to do with yuzutech/annotations-action? Can you give MUCH more details here?

tlfbrito commented 2 years ago

Sorry but what has SQLFluff got to do with yuzutech/annotations-action? Can you give MUCH more details here?

hmmm well it's used together with SQLFluff, as it's described here: https://github.com/sqlfluff/sqlfluff-github-actions/tree/main/menu_of_workflows/drizly

If you run this Github Action as it's described here: https://github.com/sqlfluff/sqlfluff-github-actions/blob/main/menu_of_workflows/drizly/lint_sqlfluff.yml it's going to fail with the SQLFluff 0.9.0.

tunetheweb commented 2 years ago

Ah apologies. Missed this was in the sqlfluff-github-actions repo, rather than the main one.

Still, I'm not aware of any changes in this space and running the following:

sqlfluff lint --format github-annotation --annotation-level failure --nofail test.sql

Produces valid JSON.

Can you see what the differences is in the JSON produced by SQLFluff 0.9.0 and the previous version by running above command on your SQL?

jpy-git commented 2 years ago

@tunetheweb In the docs it recommends using the yuzutech action and there's a flag in the CLI that outputs the results in "github-annotation" format, which is some special JSON format for this yuzutech action not a general Github one.

I dislike that that we're tied to using this yuzutech action for Github annotations. We can make a simple Github action using a regex problem matcher (https://github.com/mheap/problem-matcher) to parse the normal sqlfluff output and create github annotations (I do this for SQLFluff, flake8, mypy, and detect-secrets at work).

^I'll pull together a POC on this

tunetheweb commented 2 years ago

@tunetheweb In the docs it recommends using the yuzutech action

"recommends" is strong IMHO. My understanding is this is a collection of GitHub Actions that SQLFluff users have implemented more as a showcase of what can be done, than something we will necessarily provide support on — especially when it involves other third-party actions we have no control over. I also think we should update the README of this repo to reflect that.

And while I agree the yuzutech action may be over kill here, I'm still curious as to what's changed in SQLFluff 0.9.0 to cause this issue. The fact that even empty JSON causes this error suggests the issue is with the yuzutech action but weird that it's coupled to SQLFluff 0.9.0. Either way need more info from OP to make any progress here.

tlfbrito commented 2 years ago

Sorry but what has SQLFluff got to do with yuzutech/annotations-action? Can you give MUCH more details here?

Also no need to be so defensive, I can close the issue if you sleep better that way

tunetheweb commented 2 years ago

Apologies again for the curtness of my initial response @tlfbrito . As I say I made a mistake and failed to notice this was in the SQLFluff actions repo.

Saying that, as per above, I still do not believe there is enough info in the original request to investigate further.

jpy-git commented 2 years ago

@tlfbrito let's keep this issue focussed...

As @tunetheweb was hinting, can you provide:

thanks 😄

jpy-git commented 2 years ago

@tunetheweb as a takeaway could we look at adding issue templates to the other SQLFluff org repos similar to what we have in the main one? Happy to help with some of these

jpy-git commented 2 years ago

"recommends" is strong IMHO.

Re: the "recommends" point, it's referenced directly in the main docs here and as I said the --format github-annotation option is specific to the yuzutech format so I would say that we are pretty strongly implying that this is the recommended approach atm.

(I'll make a separate issue for discussing the recommended approach though)

tunetheweb commented 2 years ago

How many times can I be wrong in the one issue 😞

Still, on the plus side, 2021 is nearly at an end! 😀

tlfbrito commented 2 years ago

@tlfbrito let's keep this issue focussed...

As @tunetheweb was hinting, can you provide:

  • SQL (or a minimum reproducible example) that is causing the issue. Also include some info on your config, dialect, etc.
  • Check if this issue also happens for SQLFluff 0.8.2, I'm also not aware of any changes to the github actions json on our end so would be helpful to narrow things down (there's always the possibility that something has changed on the yuzu end).

thanks 😄

It seems that the file it's contains "15:03:10 Partial parse save file not found. Starting full parse.", this only happen with the 0.9.0 version of SQLFluff which I had to use because we are also using DBT 1.0.0

tunetheweb commented 2 years ago

That looks to be a DBT error: https://github.com/dbt-labs/dbt-core/issues/3886

Also searched and can't find it in our code base.

Is there anyway to configure DBT so as not so add this (I don't use DBT)?

jpy-git commented 2 years ago

yeah in that case it sounds like a fatal dbt error rather than anything to do with annotations. Surely the same thing would happen if you just ran the sqlfluff lint function normally (via the CLI)?

alanmcruickshank commented 2 years ago

I'm now also be experiencing the same issue and I'm pretty sure sqlfluff is causing it - or at least it's implementation of the dbt templater.

Currently having difficulty recreating it outside of the environment of the github action - but I'm working on a failing test case to demonstrate the issue.

alanmcruickshank commented 2 years ago

Ok, I think I've got a workaround and a diagnosis.

@tunetheweb is right that the command isn't being emitted by SQLFluff directly. SQLFluff uses dbt under the hood, which is outputting to stdout directly itself (see dbt functions.py). Because we run sqlfluff lint in the github action, and redirect the stdout to file (using >), we're picking up both the intended stdout of SQLFluff and the unintended stdout of dbt. In most of the test cases I've tried to arrange, everything was looking fine, because all our test cases collect the stdout of sqlfluff - and that's not the problem here. dbt is outputting directly to stdout and bypassing sqlfluff entirely.

To make things more unhelpful, dbt is using a logger called default_stdout rather than dbt. and so it's not immediately obvious that things are coming from there.

I'm going to lodge an issue with dbt asking how we can suppress their log output.

In the meantime, I've got a workaround (@tlfbrito) which is working on our project:

alanmcruickshank commented 2 years ago

As an update, the dbt team are working on https://github.com/dbt-labs/dbt-core/issues/3451, which should provide us with a solution to this problem.

alanmcruickshank commented 1 year ago

If anyone is watching this one - there's an alternative solution in progress here: #5070