Bind out cross-process lock with unit tests. #519
Conversation
| try: | ||
| new_lock = crt_instance_lock_acquire(nonce_str) | ||
| self.fail("Acquiring a lock by the same nonce should fail when it's already held") | ||
| except RuntimeError as e: |
There was a problem hiding this comment.
Does the CRT have any formalized error hierarchy in Python? I haven't dug too deeply yet. My only concern here is RuntimeError is very generic so it will be difficult to do control flow handling with this without message introspection.
There was a problem hiding this comment.
@graebm any thoughts here?
There was a problem hiding this comment.
It's not great. It's the same problem we have in every language binding. We didn't code-gen error classes, so catching specific CRT errors means users must catch a RuntimeError and introspect it.
In this specific case, we could dance around the issue and just return None or False if the C function returns NULL, since we do often expect it to fail 🤷♀️
There was a problem hiding this comment.
I'm not happy about this. I really want to do a mildly-breaking-change to all our bindings and address this issue. But it's non-trivial :|
add better documentation. Co-authored-by: Nate Prewitt <[email protected]>
| try: | ||
| new_lock = crt_instance_lock_acquire(nonce_str) | ||
| self.fail("Acquiring a lock by the same nonce should fail when it's already held") | ||
| except RuntimeError as e: |
There was a problem hiding this comment.
It's not great. It's the same problem we have in every language binding. We didn't code-gen error classes, so catching specific CRT errors means users must catch a RuntimeError and introspect it.
In this specific case, we could dance around the issue and just return None or False if the C function returns NULL, since we do often expect it to fail 🤷♀️
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.