toolshed/organising
7
1
Fork 0
Code Issues 100 Pull Requests Releases Activity

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

New Issue
Closed
opened 2023-10-03 20:20:04 +00:00 by 3wordchant · 10 comments
3wordchant commented 2023-10-03 20:20:04 +00:00
Owner
Copy Link

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
Owner
Copy Link

@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
Author
Owner
Copy Link

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
Owner
Copy Link

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
Author
Owner
Copy Link

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
Owner
Copy Link

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
Author
Owner
Copy Link

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
Owner
Copy Link

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

@3wordchant ah sorry, going in circles 😅 that bullet-point one looks legit!
🚀 1
3wordchant commented 2025-08-12 10:40:33 +00:00
Author
Owner
Copy Link

OK makingitso

OK makingitso
🎉 1
3wordchant commented 2025-08-12 10:49:10 +00:00
Author
Owner
Copy Link

@decentral1se am I correct that we still want to just unconditionally bail in upgrade and rollback which have no --chaos option?

@decentral1se am I correct that we still want to just unconditionally bail in `upgrade` and `rollback` which have no `--chaos` option?
decentral1se commented 2025-08-12 10:51:43 +00:00
Owner
Copy Link

@3wordchant i believe so? If the code is doing that then probably that is what people expect 🤔

@3wordchant i believe so? If the code is doing that then probably that is what people expect 🤔
decentral1se moved this to Done in Abra v0.11.x on 2025-08-12 12:20:07 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
abra
Everything to do with abra
abra-gandi
Abra Gandi plugin
awaiting-feedback
Ping/pong on comms
backups
bug
Something is not working
build
Go build related issues
ci/cd
Getting the robots into the mix
community organising
Opening this thing up
contributing
Contributors stuff
coopcloud.tech
Our main website
democracy
Democratic decision making
design
Design thinking required
documentation
Let's write things together
duplicate
This issue or pull request already exists
enhancement
New feature
finance
Money things
funding
Anything related to grant funding
good first issue
Easy start with development
help wanted
Need some help
installer
Installation related issues
kadabra
performance
Performance related
proposal
Large change which requires feedback & decisin making
question
More information is needed
recipes.coopcloud.tech
Our recipes catalogue
security
Securing our shit
test
Unit or integration test suite
wontfix
This won't be fixed
No Label
abra
bug
Milestone
No items
No Milestone
Projects
Clear projects
No project Abra v0.11.x
Assignees
Clear assignees
3wordchant aadil abra-bot ammaratef45 Apfelwurm carla cas coopcloud cyrnel decentral1se fauno iexos jade jmakdah2 joe-irving kawaiipunk knoflook kolaente lambdabundesverband linnealovespie moritz notplants p4u1 pharaohgraphy renovate-bot ripclap simon sixsmith trav val yksflip
No Assignees 3wordchant
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: toolshed/organising#497
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?