add general-purpose execute function to outbound-redis
#1134
Conversation
This file contains 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
By popular request, this function should allow executing any arbitrary Redis command and retrieving its result, if any. Ideally, we'd support a statically typed interface for every command, but that would be a pretty huge effort, so we settle for dynamic typing for now. Once we have TCP socket support, users will be able to use their favorite Redis client library and enjoy static typing at that level, if applicable. Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
|
FYI, the CI failure is due to the dreaded "no space left on device" issue running the E2E tests on Windows. |
itowlson
reviewed
Feb 14, 2023
|
Really love to see the addition of this, thanks for taking it on. Do we need a checklist for keeping track of other changes outside of this repo? Specifically thinking about other SDKs. |
radu-matei
reviewed
Feb 14, 2023
lann
reviewed
Feb 14, 2023
| @@ -76,6 +77,15 @@ pub struct RedisReport { | |||
| /// "baz"))` as the result. The host will assert that said function is called exactly once with the specified | |||
| /// arguments. | |||
| pub smembers: Result<(), String>, | |||
|
|
|||
There was a problem hiding this comment.
As a total aside, it would be interesting to run these conformance tests under tarpaulin.
lann
reviewed
Feb 14, 2023
Co-authored-by: itowlson <github@hestia.cc> Signed-off-by: Joel Dice <joel.dice@fermyon.com>
lann
reviewed
Feb 14, 2023
lann
approved these changes
Feb 14, 2023
radu-matei
approved these changes
Feb 14, 2023
The `nil` case doesn't make sense for parameters, and string arguments have different interpretations than Redis status results, so they should have different name. This also renames cases to be consistent with the names in the database WIT files and with the semantics of Redis status messages. The name `redis-result` is admittedly somewhat verbose, but I wanted to avoid clashing with Rust's `Result` type and also the HTTP `Response` type, which didn't leave me with any natural, succinct names. And, to be consistent, I went with `redis-parameter` for the other type. Naming is hard. Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
By popular request, this function should allow executing any arbitrary Redis command and retrieving its result, if any. Ideally, we'd support a statically typed interface for every command, but that would be a pretty huge effort, so we settle for dynamic typing for now. Once we have TCP socket support, users will be able to use their favorite Redis client library and enjoy static typing at that level, if applicable.
Signed-off-by: Joel Dice joel.dice@fermyon.com