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

Lookup failure exceptions #380

Open
jakebeal opened this issue Feb 10, 2022 · 2 comments
Open

Lookup failure exceptions #380

jakebeal opened this issue Feb 10, 2022 · 2 comments
Labels
SBOL-utilities Issues to migrate to https://github.com/SynBioDex/SBOL-utilities
Milestone

Comments

@jakebeal
Copy link
Contributor

Currently, lookup returns None when it doesn't find anything, which we've known might be a problem.

# TODO: Should the lack of a parent raise an error?

In fact, this does tend to lead to opaque errors, like iGEM-Engineering/iGEM-distribution/issues/222
I would like to have it raise exceptions instead, of two classes:

  • Missing child objects - this is a serious error, and should never happen
  • Missing toplevel objects - this is legal state, but needs to be handled as an exception situation, allowing the calling function to decide on a context-appropriate action to take (e.g., record the outgoing line, fetch the missing object, abort its operation, declare an error)

Both of these should be child of an exception class that covers both, so that one can decide whether to catch either or both.

I am divided on whether to make this backward compatible with an optional or not. On the one hand, I'm certain that not making it backward compatible will cause lots of things to break. On the other hand, this is our last chance to rip off the bandaid before 1.0 deployment.

@jakebeal jakebeal added this to the 1.0 milestone Feb 10, 2022
@tcmitchell
Copy link
Collaborator

An alternate approach is to document the lookup method as returning either an instance of sbol3.Identified or None if the object is not found. Callers should be responsible for checking the returned value before proceeding with it.

The stack trace in iGEM-Engineering/iGEM-distribution#222 shows code that is using the value returned from lookup without checking to see if an object was found. That feels like an application error. And without proper method documentation on lookup, it is understandable.

I think what we have to decide is whether the inability to lookup a URI via the lookup method is an exceptional condition or simply a possible outcome. Since there is nothing in the SBOL 3 specification that requires a reference to resolve within a document I think it falls into the latter camp of "possible outcome", rather than an exceptional condition. Others may view the situation differently.

@tcmitchell tcmitchell added the SBOL-utilities Issues to migrate to https://github.com/SynBioDex/SBOL-utilities label Feb 10, 2022
@tcmitchell tcmitchell modified the milestones: 1.0, 1.1 Feb 10, 2022
@jakebeal
Copy link
Contributor Author

Planned approach: two SBOL utilities functions, get_child and get_toplevel (so similar)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SBOL-utilities Issues to migrate to https://github.com/SynBioDex/SBOL-utilities
Projects
None yet
Development

No branches or pull requests

2 participants