--chaos should warn instead of error on recipe lint checks #497

New Issue

2023-10-03T20:20:04Z

3wordchant commented

2023-10-03 20:20:04 +00:00

Deep in the hacking mines - it feels like the intention of specifying --chaos should turn recipe lint errors into warnings

Deep in the hacking mines - it feels like the intention of specifying `--chaos` should turn recipe lint errors into warnings

3wordchant added the bug label 2023-10-03 20:20:04 +00:00

decentral1se commented

2023-10-03 23:54:57 +00:00

@3wordchant True!

The LintForErrors logic errors out on first linting error, would you like to see all errors turned into warnings and outputted? That might be a good change to make in general, to see all possible errors/warnings at once instead of running into them one by one?

Altho, probably the simplest thing to do on the code side would be to warn and skip linting altogether when --chaos. Unless you think it's handy & worth it to see your potential errors while --chaos'in ❔

@3wordchant True! The `LintForErrors` logic errors out on first linting error, would you like to see all errors turned into warnings and outputted? That might be a good change to make in general, to see all possible errors/warnings at once instead of running into them one by one? Altho, probably the simplest thing to do on the code side would be to warn and skip linting altogether when `--chaos`. Unless you think it's handy & worth it to see your potential errors while `--chaos`'in ❔

decentral1se added the abra label 2024-03-27 06:24:34 +00:00

3wordchant self-assigned this 2025-06-30 15:02:36 +00:00

decentral1se added this to the Abra v0.11.x project 2025-07-08 10:07:11 +00:00

3wordchant commented

2025-08-11 10:58:31 +00:00

OK I think I have it, in calix/497:

➜ abra app deploy test.example.com  
FATA lint error in test configs: "traefik routing enabled" failed lint checks (R010)
lint error in test configs: "all services have images" failed lint checks (R011)

➜ abra app deploy test.example.com  --chaos
WARN lint error in test configs: "traefik routing enabled" failed lint checks (R010)
lint error in test configs: "all services have images" failed lint checks (R011)
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃           NEW DEPLOY OVERVIEW           ┃
┃                                         ┃
┃ DOMAIN                test.example.com  ┃
┃ RECIPE                test              ┃
┃ SERVER                example.com       ┃
┃ CONFIG                compose.yml       ┃
┃                                         ┃
┃ CURRENT DEPLOYMENT    N/A               ┃
┃ ENV VERSION           N/A               ┃
┃ NEW DEPLOYMENT        b42f7841          ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
? proceed? No
FATA deployment cancelled

Multi-line display of the linting errors is a little wacky, but functional. @decentral1se what do you reckon UI-wise?

OK I think I have it, in `calix/497`: ``` ➜ abra app deploy test.example.com FATA lint error in test configs: "traefik routing enabled" failed lint checks (R010) lint error in test configs: "all services have images" failed lint checks (R011) ➜ abra app deploy test.example.com --chaos WARN lint error in test configs: "traefik routing enabled" failed lint checks (R010) lint error in test configs: "all services have images" failed lint checks (R011) ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ NEW DEPLOY OVERVIEW ┃ ┃ ┃ ┃ DOMAIN test.example.com ┃ ┃ RECIPE test ┃ ┃ SERVER example.com ┃ ┃ CONFIG compose.yml ┃ ┃ ┃ ┃ CURRENT DEPLOYMENT N/A ┃ ┃ ENV VERSION N/A ┃ ┃ NEW DEPLOYMENT b42f7841 ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ ? proceed? No FATA deployment cancelled ``` Multi-line display of the linting errors is a little wacky, but functional. @decentral1se what do you reckon UI-wise?

❤️ 1

decentral1se commented

2025-08-11 13:14:37 +00:00

Looking good @3wordchant! I'm not actually sure why we even do lint error in test configs: ...? It seems like double logging since we have the line above it already? Sure do whatever is handiest on your side. I'll be happy to review whatever.

Looking good @3wordchant! I'm not actually sure why we even do `lint error in test configs: ...`? It seems like double logging since we have the line above it already? Sure do whatever is handiest on your side. I'll be happy to review whatever.

3wordchant commented

2025-08-11 13:40:26 +00:00

I'm not actually sure why we even do lint error in test configs: ...? It seems like double logging since we have the line above it already?

Yeah, down to remove duplication. So you reckon the output should be like this (1):

➜ abra app deploy test.example.com  
FATA lint error in test configs: 
traefik routing enabled" failed lint checks (R010)
lint error in test configs: "all services have images" failed lint checks (R011)

Or like this (2):

➜ abra app deploy test.example.com  
FATA: traefik routing enabled" failed lint checks (R010)
lint error in test configs: "all services have images" failed lint checks (R011)

> I'm not actually sure why we even do `lint error in test configs: ...`? It seems like double logging since we have the line above it already? Yeah, down to remove duplication. So you reckon the output should be like this (1): ``` ➜ abra app deploy test.example.com FATA lint error in test configs: traefik routing enabled" failed lint checks (R010) lint error in test configs: "all services have images" failed lint checks (R011) ``` Or like this (2): ``` ➜ abra app deploy test.example.com FATA: traefik routing enabled" failed lint checks (R010) lint error in test configs: "all services have images" failed lint checks (R011) ```

decentral1se commented

2025-08-12 05:20:43 +00:00

Sorry, yeh this is going a bit into refactor territory... but all these outputs seem really noisy to me now that I see them again. No strong feelings on it though, so proceed as you see fit. I'm also looking into the code, I'm like of course saying to myself: "why did I do this" 😂

I was thinking why can it not just be:

➜ abra app deploy test.example.com  
FATA: traefik routing enabled" failed lint checks (R010)

So, just detect that there is a critical failure and fail without further duplicate logging. And actually, now that I look at LintForErrors in pkg/lint/recipe.go, I see that it bails out on the first error and doesn't show all the possible errors, so that could even be a bonus?

Slipping out the backdoor now...

Sorry, yeh this is going a bit into refactor territory... but all these outputs seem really noisy to me now that I see them again. No strong feelings on it though, so proceed as you see fit. I'm also looking into the code, I'm like of course saying to myself: "why did I do this" 😂 I was thinking why can it not just be: ``` ➜ abra app deploy test.example.com FATA: traefik routing enabled" failed lint checks (R010) ``` So, just detect that there is a critical failure and fail without further duplicate logging. And actually, now that I look at `LintForErrors` in `pkg/lint/recipe.go`, I see that it bails out on the first error and doesn't show all the possible errors, so that could even be a bonus? Slipping out the backdoor now...

decentral1se moved this to In Progress in Abra v0.11.x on 2025-08-12 05:23:14 +00:00

3wordchant commented

2025-08-12 10:23:21 +00:00

I was thinking why can it not just be:

Because my output has two different lint errors:

traefik routing enabled (R010)
all services have images (R011)

Because you said:

would you like to see all errors turned into warnings and outputted?

And I thought "yes that would be neat".

I agree having 2x "failed lint checks" is nonideal, how about:

➜ abra app deploy test.example.com  
FATA recipe failed lint checks:
* traefik routing enabled (R010)
* all services have images (R011)

> I was thinking why can it not just be: Because my output has two different lint errors: * traefik routing enabled (R010) * all services have images (R011) Because you said: > would you like to see all errors turned into warnings and outputted? And I thought "yes that would be neat". I agree having 2x "failed lint checks" is nonideal, how about: ``` ➜ abra app deploy test.example.com FATA recipe failed lint checks: * traefik routing enabled (R010) * all services have images (R011) ```

decentral1se commented

2025-08-12 10:39:24 +00:00

@3wordchant ah sorry, going in circles 😅 that bullet-point one looks legit!

🚀 1

3wordchant commented

2025-08-12 10:40:33 +00:00