Raise targeted exceptions in `validate()` method
helrond opened this issue · 5 comments
Currently the methods called by validate() all raise a ProfileValidationError, which make it hard to target specific errors.
I'd like to implement more specific exceptions, so that, for example, validate_bagit_profile_accept_bagit_versions() would raise an AcceptBagItVersionsValidationError, which would be a subclass of ProfileValidationError.
One point of discussion: the validate() function as currently written seems designed to produce a comprehensive list of errors, rather than raising the first exception it encounters. If my read on that is true, it may be necessary to implement an additional validation method that would raise exceptions.
Currently the methods called by
validate()all raise aProfileValidationError, which make it hard to target specific errors.
ProfileValidator.validate, which validates a bag using the profile, currently catches all of those exceptions and doesn't re-raise them, so you will not see them. Only the functions used to validate the profile throw uncaught exceptions that you could handle yourself. I didn't write this part, but presumably profile validation errors raise an exception so that one does not blithely forge ahead validating a bag with an invalid profile.
I would add that the exceptions can be distinguished by the associated value, which corresponds to argument to self._fail. Further, the variable msg in the scope of the for loop (and, thus, the current exception handler) could be use for the same targeting.
I'd like to implement more specific exceptions, so that, for example,
validate_bagit_profile_accept_bagit_versions()would raise anAcceptBagItVersionsValidationError, which would be a subclass ofProfileValidationError.
That would be one way to do it, but once an exception is raised, control will pass back to the scope containing the handler, so ProfileValidator.validate would not run to completion. Is there any particular reason why it would not be acceptable to allow validate to complete and then inspect the report?
Because exceptions are such a blunt instrument, I would be interested to hear your use case(s) to see if there other approaches that get you to what you need.
One point of discussion: the
validate()function as currently written seems designed to produce a comprehensive list of errors, rather than raising the first exception it encounters. If my read on that is true, it may be necessary to implement an additional validation method that would raise exceptions.
Here are a couple of alternatives that spring to mind:
- Have
ProfileValidator.validateaccept an optional kwarg containing a boolean that tells it whether it should re-raise any exception in the handler after it is caught. - Have
ProfileValidator.validateaccept an optional kwarg that tells it for whichmsgvalues it should re-raise an exception in the handler after it is caught.
Hi @tdilauro thanks for your response. My main use case here is that I'm using bagit-profiles in a Django application, and want to grab and persist validation error messages to the database. Based on what you're saying above, it seems like that should be possible right now by targeting the report attribute of an instantiated ProfileValidator. Is that true?
Close. There is no ProfileValidator. The Profile object has a report attribute that is initially None, but gets set to a new ProfileValidationReport at the beginning of each run of Profile.validate. You should capture either a reference to or the contents of that report as soon as validate completes, as subsequent calls to validate will overwrite the report attribute.
Have a look at ProfileValidationReport and this test to get a better sense of how it works. Currently, if validate returns success, then there are no error messages in the report.
Got it - I think that takes care of my use case, thank you!
Digging into this a bit more it looks like we were using an older version of the library, which didn't help. I've updated to 1.3.1 now and things are working much better.