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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial implementation for embedded datastore #500
Initial implementation for embedded datastore #500
Changes from 25 commits
a0c90e4
4fcede3
76cb6d0
6b86771
85b9d5b
b762a98
8d8b016
fba7c1b
56b8921
a4d51f3
ff3e8f2
3ba987c
4a22d20
ea5f996
6a98134
3119e98
5d8022a
811eef7
ec31277
20b1257
8e7f7be
be913db
17d517a
6df1528
3015104
0475fac
3c3891c
b98ef2a
2fe0777
aa6eb7a
c1fa42d
a1c5c63
3d00abf
a7ecfe4
5849a86
9c0134e
31eb475
37670e8
96cd8f6
6599eab
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could lead to some level of confusion. It confused me. Mainly because this implies that there may be other options for an 'embedded' datastore (and slightly because it is etcd which may be slightly confusing as thats the example we use for an external datastore too).
From what I can see this means that we are building in our own etcd alongside dqlite and either can be used at deploy-time. It isn't entirely clear what each will be used for.
So in short, I think we need a much more detailed explanation document which covers the datastore, which we can then refer to from here and keep the explanations where they need to be, freeing this up to be a more straightforward howto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed embedded seems unclear to end users. dqlite is also positioned as an embedded database so if the user selects embedded as a datastore does it mean he wants dqlite or etcd? I think the intention here is to let users slect the option of a "managed-etcd". With "managed-etcd" it is clear you do not want "managed-dqlite" or "external-etcd".
Also the word "embedded" leads you to believe that etcd is embedded into k8s whereas in reality we keep it in its own service. The end user probably does not know that the correct term used among engineers doing the implementation of that that service is that they use an embedded etcd.