Simplify Command Map #3345
Merged
Simplify Command Map #3345
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e. Fix misleading indentation in serverDaemon. Remove repeated error-handling code from ServerDaemon. Signed-off-by: Jeremiah Corrado <jeremiah.corrado@hpe.com>
Signed-off-by: Jeremiah Corrado <jeremiah.corrado@hpe.com>
Signed-off-by: Jeremiah Corrado <jeremiah.corrado@hpe.com>
Merged
bmcdonald3
approved these changes
Jun 26, 2024
Contributor
bmcdonald3
left a comment
bmcdonald3
left a comment
There was a problem hiding this comment.
Looks good to me, great savings of ~350 lines, nice work. Especially like the new way that binary payloads are handled.
stress-tess
approved these changes
Jun 26, 2024
Member
stress-tess
left a comment
stress-tess
left a comment
There was a problem hiding this comment.
This looks great!! nice work jeremiah
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to these changes, Arkouda had three command maps:
commandMaphandled the vast majority of commands,commandMapBinaryhandled commands that return abytesinstead of amsgTuple, andcommandMapArrayhandled one command that took a binary payload as an extra argument.This PR unifies the command maps by folding the
bytesreturn value intomsgTupleand the binary payload argument intoMessageArgs. As such, there is only one command map and all message handlers have the same signature. No changes to message serialization/deserialization on the frontend were required.Also makes some ergonomic improvements for initializing a
MsgTupleand simplifies error handling in ServerDaemon.