RSDK-4646 Add NoCaptureToStoreError to SDK #392
Conversation
|
Tested locally that this error is able to be used in a modular component, and is caught and handled in the existing RDK collector code (thanks for the sdk help @njooma !) |
kaywux
changed the title
src/viam/errors.py
Outdated
| """ | ||
|
|
||
| def __init__(self): | ||
| self.message = "no capture from filter module" |
There was a problem hiding this comment.
@kaywux Does this work based on the text of the error? If so, it would be useful to add an inline comment here warning future devs not to change this text.
And if it is text-based, that feels very brittle to me -- to have to keep that in sync across all the SDKs. Might be worth unifying it somehow, maybe through a custom proto message that can be returned?
There was a problem hiding this comment.
Yep the error handling is partially dependent on the text - added an inline comment.
I agree that this is brittle, especially if we ever want to change the message (though no plans as of now to need to do so) - am definitely open to ideas on how to keep this in sync across repos.
In terms of the custom proto message: are you suggesting returning a custom Response for this case instead of an error, or storing the error string content in a proto which the SDKs could import to build an error with? If it's the former - that would be pretty involved since we'd need to change every API that's touched by a collector to handle this response; potentially worth doing in the future but probably not for V0 of this feature. Not sure if I know how the latter would work since protos (as far as I've seen) are used to specify the structure of an object and not store any value - let me know if I'm misunderstanding something!
There was a problem hiding this comment.
No misunderstanding! I was initially thinking the former but I haven't looked to deeply into using enums/statuses to convey the correct error state. But regardless, I do think that it's probably outside the scope of this PR. I do think that if we don't do it now, though, it won't ever be done (just one of those things that will never get prioritized) and it will remain a potential fail point.
But with your new comment, hopefully no one changes it :)
To filter captured data, the data management collectors check for a custom grpc error. Expose this error in the SDK so that users coding in python can return this error in their modular resource code.