This repository has been archived by the owner on Mar 11, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Go Language Support Milestone v0.2.0
Really great works @marcellanz |
🎉 🚀 Very cool! Awesome work. |
Codecov Report
@@ Coverage Diff @@
## master #43 +/- ##
===========================================
+ Coverage 44.24% 66.12% +21.87%
===========================================
Files 9 32 +23
Lines 513 1417 +904
===========================================
+ Hits 227 937 +710
- Misses 241 354 +113
- Partials 45 126 +81
Continue to review full report at Codecov.
|
Awesome @marcellanz!! |
Wow. Well done, @marcellanz, well done. |
chongkai
suggested changes
Nov 2, 2020
sleipnir
reviewed
Nov 10, 2020
🎉 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #41
Features and changes for the "Go Support 0.2.0" release:
devcontainer.json
SupportCRDT support has been added as the second state model supported. Together with the Eventsoured implementation, stream runners where introduced which enables multiple entities of the same id to be run at the same time. The first (kind of naive) implementation v0.1.x had a bug where solely one entity of the same id where able to run at the same time.
The Eventsourced support was re-implemented using the CRDT stream runner and its way to run a messages stream. Similar, error handing was adopted and also the new atomic command handling error behaviour with entity restart support has been added.
The Integration Tests for the CRDT implementation follow the new model-based approach of the Cloudstate TCK. They run with an in-memory gRPC Client and Server and are written in Go. These tests are yet not available in the Cloudstate TCK and complement them having local functional tests and also tests not done by the TCK itself.
The Protobuf cleanup for go packages where necessary because the
go_package
used so far, where incompatible and incorrect. This is fixed in this milestone and also fixed in the main Cloudstate repository.With Eventsoured TCK Support also Eventsourced effects and forward support was correctly implemented and validated with the TCK.
A Significant test coverage increase was achieved with about 80 integration- and 150 unit-tests in this milestone.
Updated documentation and CRDT documentation is done with the new Antora based template.
The new API has been used in different contexts of available examples and public use of cloudstate. One is the Dapr contrib model support which uses a CRDT state model. This branch https://github.com/mrcllnz/components-contrib/tree/feature/cloudstate_crdt_support implements the Dapr Cloudstate component with the new Go API.
With the New API of this milestone, the API interfacing cloudstate instances as well as handling state model have changed slightly. For event sourcing, one is the removal of the
cloudstate.EventEmitter
type to be embedded with every entity, instead theeventsourced.EntityHandler
has to be implemented providingHandleCommand
andHandleEvent
methods. Embedding a type as part of an API seemed a bit too awkward. With this change, the Gocontext.Context
typed parameter of command and event handlers have been changed toeventsource.Context
that embeds acontext.Context
. This simply prevents an unfortunate API likewhere two context parameters are provided, one most of the time never used. Instead, the
context.Context
context is embedded and available through the API. Embeddingcontext.Context
is discouraged but not completely disallowed. The topic is well known in the Go community and there is work being done to relax recommendation against putting Contexts in structs. The narrative forcontext.Context
is basically to discourage sharing state by embedding a context in structs and use it instead, to get the contexts passed along chains of function calls and use them with their purpose. We could have embedded the cloudstateeventsourced.Context
within acontext.Context
with context.WithValue as other similar projects do it, but I found it too much of a stretch to get them again out again of acontext.Context
context.Second, the
Snapshotter
andSnapshotHandler
interfaces were combined into the optionaleventsourced.Snapshooter
interface.Regarding error handling, the in Cloudstate otherwise well known
context.Fail
method to be used to signal the failure of a running context, was replaced with the in Go idiomatic return of an error as a value to be returned explicitly. This transformed the following kind of awkward code snipped:into:
which is much more explicit as it embraces the principle of Errors are values in Go.
Regarding the API of CRDT types, a decision was made how to represent, or better said encode, values in structures like maps, sets and registers. We follow the interface{} says nothing proverb, where we simply don't put empty interface typed values into a map, set or register. Until probably 2021, when Go gets generics-support, we put
*any.Any
values into maps, sets and registers. There is no magic of how to find out what it is in a map, the type used forORMaps
andORSets
as well asLWWRegisters
. Go maps actually are generic types, where the compiler generates code for declared types likemap[string]*shopping.Cart
but this is not available in a dynamic way like we need it for our maps, sets and registers. While*any.Any
types are kind of like empty interfaces they are more explicit that anything else in the context of Cloudstate, Cloudstate Go users know that they implement gRPC services and that their representation have a relationship to be*any.Any
types.Second, as we put
*any.Any
values into maps, sets and registers, we have to decide when to marshal (encode) values into*any.Any
values. We could have chosen to encode values at times when cloudstate protocol replies where prepared to be sent (ActionReplies, CRDT State and Deltas) or right when they're inserted into CRDT types. I've chosen the approach to fail fast (thanks Joe) and put the burden to the API user. It's not that much of a hassle I think, it also shows how explicit we use and implement state-model as we don't hide them (actually we can't) inside an implementation of aSet
orMap
with then some implicit or explicit restrictions other Language Support libraries (rightfully in their field) have done.There are occasions where this decision brings an additional decode and probably encode step like the
getCart
method in the CRTD shopping cart example:as we have to get values out of a CRDT type again and, to return them in the form of a gRCP service return type. If performance-critical, this can be explicitly optimized. The next iteration of the support library will show if that was the right decision or if it has to be adjusted. I did not want to optimize without empirical data. Also,
any.Any
types for many Cloudstate state model types are the encoded format that gets to be known for the proxy. This said, the second Go proverb chosen in this issue that says: Clear is better than clever is reflected and implemented in this PRs work.