[python] check encoding version in all open code paths#4055
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4055 +/- ##
==========================================
+ Coverage 89.19% 89.28% +0.08%
==========================================
Files 57 59 +2
Lines 6903 7063 +160
==========================================
+ Hits 6157 6306 +149
- Misses 746 757 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
bkmartinjr
changed the title
| ) | ||
| if _read_soma_type(handle) != cls.soma_type: | ||
| raise SOMAError( | ||
| "Unexpected SOMA metadaa encoding - unable to determine object type." |
There was a problem hiding this comment.
I'm not sure this is the right error. Unless I'm misunderstanding this code, it looks like we were able to determine the type - it just wasn't the type we were expecting.
There was a problem hiding this comment.
You are correct - it is either a sign that we have a logic bug, OR the data was corrupted in some weird way.
I'll update the error message.
jp-dark
left a comment
jp-dark
left a comment
There was a problem hiding this comment.
I have some change requests around the error messages. I'm open to other wording than what I provided here, but I would like to make the errors more helpful if possible.
|
|
||
| if obj_type is None: | ||
| raise SOMAError( | ||
| f"stored TileDB object does not have {SOMA_OBJECT_TYPE_METADATA_KEY!r}" |
There was a problem hiding this comment.
| f"stored TileDB object does not have {SOMA_OBJECT_TYPE_METADATA_KEY!r}" | |
| f"Cannot access stored TileDB object with TileDB-SOMA. The object is missing " | |
| f"the required '{SOMA_OBJECT_TYPE_METADATA_KEY!r}' metadata key." |
| f"stored TileDB object {SOMA_OBJECT_TYPE_METADATA_KEY!r}" | ||
| f" is {type(obj_type)}" |
There was a problem hiding this comment.
| f"stored TileDB object {SOMA_OBJECT_TYPE_METADATA_KEY!r}" | |
| f" is {type(obj_type)}" | |
| f"Cannot access stored TileDB object with TileDB-SOMA. The metadata key " | |
| f"'{SOMA_OBJECT_TYPE_METADATA_KEY!r}' has unexpected type {type(obj_type)}." |
| ) | ||
| if encoding_version is None: | ||
| raise SOMAError( | ||
| f"stored TileDB object does not have {SOMA_ENCODING_VERSION_METADATA_KEY}" |
There was a problem hiding this comment.
| f"stored TileDB object does not have {SOMA_ENCODING_VERSION_METADATA_KEY}" | |
| f"Cannot access stored TileDB object with TileDB-SOMA. The object is missing " | |
| f" the required '{SOMA_ENCODING_VERSION_METADATA_KEY}' metadata key." |
| f"Unsupported SOMA object encoding version {encoding_version}. The TileDB-SOMA " | ||
| f"client library needs to be updated to a more recent version." |
There was a problem hiding this comment.
| f"Unsupported SOMA object encoding version {encoding_version}. The TileDB-SOMA " | |
| f"client library needs to be updated to a more recent version." | |
| f"Unsupported SOMA object encoding version {encoding_version}. TileDB-SOMA " | |
| f"needs to be updated to a more recent version." |
This one was mine. In retrospect I think specifying "client library" is more likely to be confusing than helpful.
|
@jp-dark - I incorporated your message changes (almost literally, with a couple of minor punctuation changes). Also tried to clarify the "unexpected" type error. I debated whether this should be an assertion or error, as it isn't something the user can handle, but most of these "bad metadata" checks are similar, so ended up leaving it as is (with clarified message) |
2 participants
Fixes SOMA-54
The
tiledbsoma.opencode path checks encoding version and generates a nice error if the object is unsupported. All other class-specific open paths failed to perform this check. This can lead to the situation where the user is able to open SOMA objects with an unsupported version, which will then fail in surprising ways elsewhere.Changes:
openwill do the same checks and generate the same errors (as needed)