Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formatting is broken #66

Closed
joeytepp opened this issue Feb 6, 2024 · 6 comments
Closed

Formatting is broken #66

joeytepp opened this issue Feb 6, 2024 · 6 comments

Comments

@joeytepp
Copy link

joeytepp commented Feb 6, 2024

Running into some issues in CI since #63 shipped. Have some complex queries that involve template variables and nested subqueries that the formatter says are incorrectly formatted, but when I format using the --yes option it just indents all the code. While it would be nice to fix this in the CLI adding the ability to skip formatting in the github actions workflow would be a good workaround for the time being.

Example pipe definition to reproduce:

NODE some_node
SQL >
    %
    SELECT
      date,
      {% if some_var == 'MAXIMUM' %}
        MAX(CASE WHEN type = 'thing' THEN some_col END) as things,
        MAX(CASE WHEN type = 'other_thing' THEN some_col END) as other_things,
        IF(things IS NULL AND other_things IS NULL, NULL, MAX(some_col)) as both_things_and_other_things
      {% elif some_var == 'MINIMUM' %}
        MIN(CASE WHEN type = 'thing' THEN some_col END) as things,
        MIN(CASE WHEN type = 'other_thing' THEN some_col END) as other_things,
        IF(things IS NULL AND other_things IS NULL, NULL, MIN(some_col)) as both_things_and_other_things
      {% elif some_var == 'AVERAGE' or not defined(some_var) %}
        avgWeighted(CASE WHEN type = 'thing' THEN some_col END, 1) as things,
        avgWeighted(CASE WHEN type = 'other_thing' THEN some_col END, 1) as other_things,
        IF(isNaN(avgWeighted(some_col, 1)), NULL, avgWeighted(some_col, 1)) as both_things_and_other_things
      {% end %}
    FROM some_tables
@alrocar
Copy link
Member

alrocar commented Feb 7, 2024

@joeytepp

Which version of ci.yaml are you using in your project? It should be v3.0.0

Apart from that, you can disable formatting for a specific node (or part of a query) like this

@joeytepp
Copy link
Author

joeytepp commented Feb 7, 2024

Our repository has the following YAML in the file .github/workflows/tinybird_ci.yml

uses: tinybirdco/ci/.github/workflows/ci.yml@main

I will use the fmt: off directive though for now. Thanks!

@joeytepp
Copy link
Author

joeytepp commented Feb 7, 2024

It turns out the -- fmt: off directive isn't working 😕

@alrocar
Copy link
Member

alrocar commented Feb 7, 2024

you can change the template to:

uses: tinybirdco/ci/.github/workflows/[email protected]

It turns out the -- fmt: off directive isn't working 😕

Could you copy/paste how you are using it? I guess it should work 🤔

@joeytepp
Copy link
Author

joeytepp commented Feb 7, 2024

I tried all the ways that were outlined here (ie top of file, in the middle of a statement) and it still didn't work. This was specifically for a query like the one in my description which the formatter currently doesn't seem to handle correctly

@alrocar
Copy link
Member

alrocar commented Mar 27, 2024

Hi @joeytepp we've fixed the issue with the CASE directive that prevent files to be correctly formatted.

It's been fixed in tinybird-cli 3.6.1.dev5 and will be released next week.

Thanks!

@alrocar alrocar closed this as completed Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants