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

feat: improve error logs when running sync/diff/validate #976

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

GGabriele
Copy link
Collaborator

@GGabriele GGabriele commented Jul 18, 2023

Currently decK doesn't return very useful and descriptive
errors when configuration files are validated. This is
particularly painful when the configuration is distributed
in several files and the error doesn't specificy which
file contains the error.

Let's take for example the following files:

$ cat services.yaml
services:
- name: svc1
  host: mockbin.org
  foo: bar

consumers:
- username: good
- name: bad

If we run decK, it will return some obscure and incomplete errors:

$ deck sync -s services.yaml -s consumers.yaml
Error: reading file: validating file content: 1 errors occurred:
        services.0: Additional property foo is not allowed

This commit is going to improve the way decK handles
configuration validation error logging in order to provide
a more useful description of the problems.
When running decK with the new functionality, the following will
be returned:

$ ./deck sync -s services.yaml -s consumers.yaml
Error: 2 errors occurred:
	reading file services.yaml: validating file content: 1 errors occurred:
	validation error: object="bar", err=services.0: Additional property foo is not allowed

	reading file consumers.yaml: validating file content: 3 errors occurred:
	validation error: object={"name":"bad"}, err=consumers.1: Must validate at least one schema (anyOf)
	validation error: object={"name":"bad"}, err=consumers.1: username is required
	validation error: object="bad", err=consumers.1: Additional property name is not allowed

@GGabriele GGabriele requested a review from a team as a code owner July 18, 2023 19:51
Currently decK doesn't return very useful and descriptive
errors when configuration files are validated. This is
particularly painful when the configuration is distributed
in several files and the error doesn't specificy which
file contains the error.

Let's take for example the following files:

```
$ cat services.yaml
services:
- name: svc1
  host: mockbin.org
  foo: bar

consumers:
- username: good
- name: bad
```

If we run decK, it will return some obscure and incomplete errors:

```
$ deck sync -s services.yaml -s consumers.yaml
Error: reading file: validating file content: 1 errors occurred:
        services.0: Additional property foo is not allowed
```

This commit is going to improve the way decK handles
configuration validation error logging in order to provide
a more useful description of the problems.
When running decK with the new functionality, the following will
be returned:

```
$ ./deck sync -s services.yaml -s consumers.yaml
Error: 2 errors occurred:
	reading file services.yaml: validating file content: 1 errors occurred:
	validation error: object="bar", err=services.0: Additional property foo is not allowed

	reading file consumers.yaml: validating file content: 3 errors occurred:
	validation error: object={"name":"bad"}, err=consumers.1: Must validate at least one schema (anyOf)
	validation error: object={"name":"bad"}, err=consumers.1: username is required
	validation error: object="bad", err=consumers.1: Additional property name is not allowed
```
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 78.57% and project coverage change: +0.02 🎉

Comparison is base (d19b622) 33.97% compared to head (a07c5d8) 33.99%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #976      +/-   ##
==========================================
+ Coverage   33.97%   33.99%   +0.02%     
==========================================
  Files         100      100              
  Lines       11906    11906              
==========================================
+ Hits         4045     4048       +3     
+ Misses       7468     7464       -4     
- Partials      393      394       +1     
Impacted Files Coverage Δ
file/validate.go 82.35% <57.14%> (-7.65%) ⬇️
file/readfile.go 85.36% <85.71%> (+0.24%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@GGabriele GGabriele added this to the 1.24.0 milestone Jul 19, 2023
@mikefero mikefero merged commit 237adbb into main Jul 19, 2023
35 checks passed
@mikefero mikefero deleted the feat/improve-error-logs branch July 19, 2023 14:38
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

Successfully merging this pull request may close these issues.

None yet

3 participants