tinybirdco / ci

Collection of configuration files that enable CI flows for Tinybird projects
MIT License
3 stars 3 forks source link

Formatting is broken #66

Closed joeytepp closed 6 months ago

joeytepp commented 8 months ago

Running into some issues in CI since https://github.com/tinybirdco/ci/pull/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 commented 8 months ago

@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 commented 8 months ago

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 commented 8 months ago

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

alrocar commented 8 months ago

you can change the template to:

uses: tinybirdco/ci/.github/workflows/ci.yml@v3.0.0

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 commented 8 months ago

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 commented 6 months ago

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!