8000 Better error message for implicit branches by erikbosch · Pull Request #437 · COVESA/vss-tools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Better error message for implicit branches #437

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 10, 2025
Merged

Better error message for implicit branches #437

merged 2 commits into from
Mar 10, 2025

Conversation

erikbosch
Copy link
Collaborator

Fixes #407

This PR adds a custom check and error if description is empty, and mention implicit branches in overlays as a possible error reason.

$ vspec export csv -s test.vspec -u tests/vspec/test_units.yaml -q tests/vspec/test_quantities.yaml -o hej.csv
[12:08:33] INFO     Loaded 'VSSQuantity', file=/home/erik/vss-tools/tests/vspec/test_quantities.yaml, elements=6               units_quantities.py:34
           INFO     Loaded 'VSSUnit', file=/home/erik/vss-tools/tests/vspec/test_units.yaml, elements=7                        units_quantities.py:34
           INFO     VSpecs loaded, amount=1                                                                                              vspec.py:130
           CRITICAL 'A.B' has 1 model error(s):                                                                                           main.py:231
                    [                                                                                                                                
                        {                                                                                                                            
                            'type': 'value_error',                                                                                                   
                            'loc': ('description',),                                                                                                 
                            'msg': 'Value error, all nodes in the final tree must have a description. Implicit branches are not allowed              
                    in final tree!',                                                                                                                 
                            'input': ''                                                                                                              
                        }                                                                                                                            
                    ]

Current output:

$ vspec export csv -s test.vspec -u tests/vspec/test_units.yaml -q tests/vspec/test_quantities.yaml -o hej.csv
[12:08:46] INFO     Loaded 'VSSQuantity', file=/home/erik/vss-tools/tests/vspec/test_quantities.yaml, elements=6               units_quantities.py:34
           INFO     Loaded 'VSSUnit', file=/home/erik/vss-tools/tests/vspec/test_units.yaml, elements=7                        units_quantities.py:34
           INFO     VSpecs loaded, amount=1                                                                                              vspec.py:130
           CR
8000
ITICAL 'A.B' has 1 model error(s):                                                                                           main.py:231
                    [                                                                                                                                
                        {                                                                                                                            
                            'type': 'missing',                                                                                                       
                            'loc': ('description',),                                                                                                 
                            'msg': 'Field required',                                                                                                 
                            'input': {'fqn': 'A.B', 'type': 'branch'}                                                                                
                        }                                                                                                                            
                    ]

@erikbosch
Better error message for implicit branches
c6b888b 
Fixes COVESA#407

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
sschleemilch
sschleemilch approved these changes Feb 14, 2025
View reviewed changes
Copy link
Collaborator
@sschleemilch sschleemilch left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine for me although i Like the current Output more. It is more explicit and contains better context info (Input)

@SebastianSchildt
Copy link
Collaborator

Is there any reason, why we can not keep the "input", and only change msg in this special case?

@erikbosch
Copy link
Collaborator Author

Is there any reason, why we can not keep the "input", and only change msg in this special case?

It seems that when you add "manual" validators as described in https://docs.pydantic.dev/latest/concepts/validators/#json-schema-and-field-validators you have two options:

  • Field validators, as currently in the PR. They are class methods so you do not get any input
  • Model validators, then you get input but not loc. You could for instance get output like below.

Changing to something like below would be quite easy. It works to give either assert or ValueError, we are currently a bit inconsistent it seems, sometimes we use assert, sometimes raise ValueError. Could not really see a pattern.

(.venv) erik@debian7:~/vss-tools$ vspec export csv -s test.vspec -u tests/vspec/test_units.yaml -q tests/vspec/test_quantities.yaml -o hej.csv
[11:05:09] INFO     Loaded 'VSSQuantity', file=/home/erik/vss-tools/tests/vspec/test_quantities.yaml, elements=6                                                      units_quantities.py:34
           INFO     Loaded 'VSSUnit', file=/home/erik/vss-tools/tests/vspec/test_units.yaml, elements=7                                                               units_quantities.py:34
           INFO     VSpecs loaded, amount=1                                                                                                                                     vspec.py:130
           CRITICAL 'A.B' has 1 model error(s):                                                                                                                                  main.py:231
                    [                                                                                                                                                                       
                        {                                                                                                                                                                   
                            'type': 'assertion_error',                                                                                                                                      
                            'loc': (),                                                                                                                                                      
                            'msg': 'Assertion failed, all nodes in the final tree must have a description. Implicit branches are not allowed in final tree!',                               
                            'input': {'fqn': 'A.B', 'type': 'branch'}                                                                                                                       
                        }                                                                                                                                                                   
                    ]                                                                                                                                                                       
(.venv) erik@debian7:~/vss-tools$

@erikbosch
Copy link
Collaborator Author

MoM:

  • Sebastian and Sebastian to review again, if changed approach if comment would be better
  • E: if you think solution in comment gives better output I will add that in a commit and push it

@erikbosch
Copy link
Collaborator Author

Updated to use model validator instead, then input info is kept.

@erikbosch
Using model validator for error instead
287d63a 
Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
@erikbosch erikbosch merged commit 7ec87b0 into COVESA:master Mar 10, 2025
5 checks passed
@erikbosch erikbosch deleted the erik_err branch March 10, 2025 08:17
cpodalak pushed a commit to cpodalak/vss-tools that referenced this pull request Mar 25, 2025
@erikbosch @cpodalak
Better error message for implicit branches (COVESA#437)
10d5839 
* Better error message for implicit branches

Fixes COVESA#407

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>

---------

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
Signed-off-by: Chaitanya Podalakuru <118563646+cpodalak@users.noreply.github.com>
Giaccomole pushed a commit to Giaccomole/vss-tools-statistics-py-scripts that referenced this pull request Jun 11, 2025
@erikbosch @Giaccomole
Better error message for implicit branches (COVESA#437)
28e920c 
* Better error message for implicit branches

Fixes COVESA#407

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>



---------

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
Giaccomole pushed a commit to Giaccomole/vss-tools-statistics-py-scripts that referenced this pull request Jun 11, 2025
@erikbosch @Giaccomole
Better error message for implicit branches (COVESA#437)
ddfe9dc 
* Better error message for implicit branches

Fixes COVESA#407

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>



---------

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sschleemilch sschleemilch sschleemilch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

v4.2.1: Mandatory Description missing in implicitly created branches in vspec2x
3 participants
@erikbosch @SebastianSchildt @sschleemilch
0