Structured error reporting and customized rendering of parameter violations #306
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It is too much in there and too specific to be done in such a generic place. It is now located in `ParametrizationErrors._render_violations_as_indented_text_list`, which is still not good, because it is used in `ParametrizationErrors.__str__` -- but a bit better, because more concentrated.
In addition to `Constraint`-provided formatting args (from `ctx` dict), a few additional ones are provided. They make, for example, the previously unconditional formatting of "causes" for an `AnyOf` constraint configurable by a paricular `Constraint` implementation.
Allows for enabling exhaustive command parameter validation. Meanwhile the default remains to fail on first error.
…vailable This can easily happen when prior single-argument validation failed.
If this is not desirable in a particular context, the error message template can be overwritten via a `WithDescription` wrapper.
If an error provides a message, use that as-is. If there is just an error code, render that together with the URL. Otherwise render the error class name and the URL to give minimal, but critical insight.
There is no reason not to have it.
This enables error displays of the following kind: ``` ❯ datalad credentials remove some=thing [ERROR ] 1 command parameter constraint violation action='remove', name=None (remove-action requirements) no credential name provided ``` given that the identifiers `name` and `action` do not appear in the command line, indication that `remove` is the effective `action` value should help to disambiguate the semantics of the remaining word.
This makes more functionality accessible for exhaustive parameter validation, and broadens the spectrum of possible error and documentation customizations for particular use cases. Demo follows in next commit.
This command has one of the most complex input specification implemented
as nested constraints. Here is a demo of a (semi-structured) error message
rendering before this change:
```
❯ datalad download wurst
[ERROR ] 1 command parameter constraint violation
spec=['wurst']
does not match any of 3 alternatives
- does not match any of 2 alternatives
- not a recognized mapping
- does not match any of 2 alternatives
- the JSON object must be str, bytes or bytearray, not list
- not a string
- does not match any of 2 alternatives
- not enough values to unpack (expected 2, got 1)
- does not match any of 2 alternatives
- Expecting value: line 1 column 1 (char 0)
- URL is missing 'scheme' component
- not '-', or a path to an existing file
```
There is a lot of information already, but clarity is lacking.
This change applied the `WithDescription` meta constraint to bring
clarity to the error messages. The new message for the same invalid
call used above now looks like this
```
❯ datalad download wurst
[ERROR ] 1 command parameter constraint violation
spec=['wurst']
does not provide URL->(PATH|-) mapping(s)
- not a single item
- not a dict, length-2-iterable, or space-delimited str
- not a URL with a path component from which a filename can be derived
- not a JSON-encoded str with an object or length-2-array
- not a list of any such item
- not a path to a file with one such item per-line, nor '-' to read any such item from STDIN
```
The custom message use the fact that the order of constraints and
therefore the order in which violations are detected is deterministic.
The respective individual message can reference prior messages such that
the result is a more cohesive whole that, importantly, can avoid
needless repetition of information.
Conceptually (and technically), although this message is composed of the
output of multiple constraints, it is neverthess still a *single*
message, produced by a *single* top-level constraint (`AnyOf`).
Therefore, we need not worry much about this information being taken
apart and thereby becoming harder to comprehend or incomplete.
The particular set of example message may inform datalad#274 and guidelines on
how constraints and their messaging should be implemented.
|
Test situation is same as #308. Let's go! |
mih
added a commit
that referenced
this pull request
Mar 10, 2023
Clean up a fix things after rushing #306
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Elevate level of possible error message customizations. This is best demo'ed using the included update of the
downloadcommand.Prior to this PR, a typical error for the violation of a complex constraint would look like this:
A technically correct, and utterly incomprehensible monster.
With the series of feature additions and adjustments in this PR, the "same" error turns into:
The improvement is noticable.
Summary of significant changes included here:
WithDescriptionsupport error message customization, using the standardformat-like templatingdatalad.runtime.parameter-violation=raise-early|raise-end-endto enable exhaustive parameter validation (still off by default)#308 is presently still contained in here.