Fall back to pickle in protocol.dumps instead of failing hard
#8447
Conversation
distributed/protocol/core.py
Outdated
| try: | ||
| frames[0] = msgpack.dumps(msg, default=_encode_default, use_bin_type=True) | ||
| except TypeError as e: | ||
| logger.warning(e) |
There was a problem hiding this comment.
I'm not sure how meaningful this is to users. Just logging the original exception appears to be an error for users but we're handling it gracefully. If anything we should log a custom message making them aware of a possible performance issue but I'm not sure if this is actually worth a warning log
There was a problem hiding this comment.
Fair enough, I'll de-escalate that to an INFO log with a custom message. I'd like to have something logged here to raise user attention and possibly for log analysis to highlight potential performance issues.
distributed/protocol/core.py
Outdated
| if isinstance(obj, (Serialize, Serialized)): | ||
| offset = len(frames) | ||
| frames.extend(create_serialized_sub_frames(obj)) | ||
| return {"__Serialized__": offset} | ||
| elif isinstance(obj, (ToPickle, Pickled)): | ||
| offset = len(frames) | ||
| frames.extend(create_pickled_sub_frames(obj)) | ||
| return {"__Pickled__": offset} | ||
| else: | ||
| encoded = msgpack_encode_default(obj) | ||
| if encoded is not obj or _is_msgpack_serializable(obj): | ||
| return encoded | ||
| obj = ToPickle(obj) | ||
| offset = len(frames) | ||
| frames.extend(create_pickled_sub_frames(obj)) | ||
| return {"__Pickled__": offset} |
There was a problem hiding this comment.
in the interest of DRY
| if isinstance(obj, (Serialize, Serialized)): | |
| offset = len(frames) | |
| frames.extend(create_serialized_sub_frames(obj)) | |
| return {"__Serialized__": offset} | |
| elif isinstance(obj, (ToPickle, Pickled)): | |
| offset = len(frames) | |
| frames.extend(create_pickled_sub_frames(obj)) | |
| return {"__Pickled__": offset} | |
| else: | |
| encoded = msgpack_encode_default(obj) | |
| if encoded is not obj or _is_msgpack_serializable(obj): | |
| return encoded | |
| obj = ToPickle(obj) | |
| offset = len(frames) | |
| frames.extend(create_pickled_sub_frames(obj)) | |
| return {"__Pickled__": offset} | |
| rv = _enode_default(obj) | |
| if rv is obj and not _is_msgpack_serializable(obj): | |
| obj = ToPickle(obj) | |
| offset = len(frames) | |
| frames.extend(create_pickled_sub_frames(obj)) | |
| return {"__Pickled__": offset} | |
| return rv |
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 27 files ± 0 27 suites ±0 9h 37m 42s ⏱️ - 22m 5s For more details on these failures and errors, see this check. Results for commit d88c407. ± Comparison against base commit 5c481dd. |
Closes #8446
pre-commit run --all-files