SY-166 Key-Value State Forwarding & SY-1832 Go Client #1041
Conversation
…-key-value-state-forwarding
…-key-value-state-forwarding
…ub.com/synnaxlabs/synnax into sy-166-key-value-state-forwarding
…-key-value-state-forwarding
Because I made changes to the distribution layer RPCs, and re-generating with a new GPRC version causes all the PBs to change. |
emilbon99
changed the title
…-key-value-state-forwarding
There was a problem hiding this comment.
- Key-value state forwarding is good after comments are all addressed.
- For the Go client, there are a few issues:
- Needs to be much much more documentation on public functions and data types. It is a public API after all.
- Need to figure out how to publish it and add a
deploy.go.yamlfile to CI. - Need to add the go tests to the
test.client.yamlCI file. - Website documentation should also include pages on reading and streaming data.
| func Wrapf(err error, format string, args ...interface{}) error { | ||
| return errors.Wrapf(err, format, args...) | ||
| } | ||
|
|
||
| // CombineErrors returns err, or, if err is nil, otherErr. if err is non-nil, otherErr | ||
| // Combine returns err, or, if err is nil, otherErr. if err is non-nil, otherErr | ||
| // is attached as secondary error. See the documentation of `WithSecondaryError()` for |
There was a problem hiding this comment.
What is WithSecondaryError()? Should also be clear what happens when a) otherErr is
nil, err is not nil and b) both errors are nil
| _, err = u.MiddlewareCollector.Exec( | ||
| freighter.Context{ | ||
| Context: ctx, | ||
| Target: address.Newf("%s.%s", target, u.ServiceDesc.ServiceName), | ||
| Target: address.Newf("%s.%s", u.TargetPrefix+target, u.ServiceDesc.ServiceName), |
There was a problem hiding this comment.
Why do you need to concatenate u.TargetPrefix and target if target was already
assigned a new value above?
| freighter.Client, | ||
| freighter.Stream, | ||
| ), err | ||
| return freighter.Context{ |
There was a problem hiding this comment.
Might be good to make a parseClientContext function here.
| @@ -109,7 +109,7 @@ const ( | |||
| operationReceiverAddr = "opReceiver" | |||
| feedbackSenderAddr = "feedbackSender" | |||
| feedbackReceiverAddr = "feedbackReceiver" | |||
| recoveryTransformAddr = "recoveryTransform" | |||
| recoveryTransformAddr = "gossipRecoveryTransform" | |||
There was a problem hiding this comment.
why did you rename this to a different pattern than all of the other consts
| // To Open a Cluster, simply use cluster.Open. | ||
| package cluster | ||
|
|
||
| import ( | ||
| "context" | ||
| "github.com/synnaxlabs/x/query" |
|
|
||
| This script can be run in conjuction with the `control_sequence.py` script to | ||
| demonstrate how a control sequence can be written in Synnax. | ||
| """ |
There was a problem hiding this comment.
This comment does not make sense here (what is control_sequence.py). And did someone ask for us to show scheduled
commands in autosequences? If not I think we should either delete these two files or
move them into the dev folder until we have the actual implementation.
| @@ -78,7 +79,11 @@ jobs: | |||
| working-directory: client/py | |||
|
|
|||
| - name: Stop Main Server | |||
| run: docker stop synnax-main && docker rm synnax-main | |||
| if: always() && steps.start_main.outcome == 'success' | |||
There was a problem hiding this comment.
Why do you need the always() here; isn't steps.start_main.outcome == 'success'
enough? And how could we get here if that steps.start_main.outcome failed, wouldn't
this test exit at that point?
| run: | | ||
| docker logs synnax-current | ||
| docker stop synnax-current | ||
| docker rm synnax-current |
There was a problem hiding this comment.
How are there no dependencies to this file? And you need to figure out how we are
going to publish the go client, and what other go packages we will be publishing because
of that.
There was a problem hiding this comment.
Looks like this could be moved to the x module
Issue Pull Request
Key Information
Description
Adds experimental support for multi-node clusters and provides an initial implementation of a Golang client.
Basic Readiness
Backwards Compatibility
Data Structures
I have ensured that previous versions of stored data structures are properly migrated to new formats in the following projects:
API Changes
The following projects have backwards-compatible APIs:
Breaking Changes