-
Notifications
You must be signed in to change notification settings - Fork 21
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
Revamp public API #79
Merged
Merged
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
dumbbell
added
documentation
Improvements or additions to documentation
enhancement
New feature or request
labels
Apr 5, 2022
Pull Request Test Coverage Report for Build 510Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
dumbbell
force-pushed
the
revisit-public-api
branch
4 times, most recently
from
April 11, 2022 17:07
9fdadf6
to
fcfe507
Compare
dumbbell
force-pushed
the
revisit-public-api
branch
3 times, most recently
from
April 20, 2022 10:05
3797c2e
to
7572146
Compare
We still expose the basic functions from `khepri`. This helps reduce the number of lines of this module.
The previous error was `{error, matches_many_nodes}`. This was confusing because it could make the caller think that the command actually found many nodes, even though the path matched a single one or none at all. The new error is: {error, {possibly_matching_many_nodes_denied, Condition}} Therefore it highlights that the possibility of matching many is denied, not the actual result. In addition, it indicates the condition in the path which caused the problem.
Now, `khepri` is the main entry point for everything public. There is no longer a high-level API in `khepri` and a lower-level API in `khepri_machine`. `khepri` provides more user-friendly functions and we will add more when we see fit. Here are a few examples: * All functions accept both native paths (`[stock, wood, <<"oak">>]`) and Unix-like paths (`"/:stock/:wood/oak"`). Note that the syntax for Unix-like paths has changed: atoms are prefixed with `:` and binaries are left as-is. * `khepri:get_data(StoreId, PathPattern)` allows to quickly get the data attached to a specific node. It's easier than going through `khepri:get(StoreId, PathPattern)` and extracting the data from the returned map. Inside transactions, `khepri_tx` provides the same API as `khepri`, except when the function does not make sense in the context of a transaction. Unix-like paths are also accepted by `khepri_tx` functions. `khepri_cluster` is a new module to expose the clustering part of the API. This was in `khepri` before and was moved to this module. It is also part of the public interface. `khepri_path` and `khepri_condition` remain part of the public API for those needing to manipulate paths. Other modules are private however. They remain visible in the documentation because understaing the internals may help sometimes. But changes in those modules may not be documented in release notes and may not be reflected in the release versions.
dumbbell
force-pushed
the
revisit-public-api
branch
from
April 20, 2022 11:44
7572146
to
d3d4ac9
Compare
The public API is now: * `khepri_payload:none/0` * `khepri_payload:data/1` * `khepri_payload:sproc/1` That said, the caller should rarely have to use them: `khepri:put/1` tries to automatically detect if and how to wrap the given value. Therefore, one should be able to just use: khepri:put(StoreId, Path, #my_record{...}). Or: khepri:put(StoreId, Path, fun() -> ... end). Only the "no payload" case has no value beside the internal one. In this case, the caller should use: khepri:put(StoreId, Path, khepri_payload:none()). Or: khepri:clear_payload(StoreId, Path). The end goal is to reduce the chance that if we change the payload records, users are forced to recompile their code. It also makes the code maintenance easier for us. Note that in the process, the records are renamed `#p_*{}`.
The public API is now provided by the new `khepri_evf` module to construct the event filters. Before: EventFilter = #kevf_tree{path = Path, props = Props}. Now: EventFilter = khepri_evf:tree(Path, Props). The reason is the same as for the payload records: if we change the event filter records, users are not forced to recompile their code. The `khepri_machine:register_trigger/5` now tries to autodetect and convert common types to event filter, thanks to `khepri_evf:wrap/1`. This is the case for native and Unix-like paths which are converted to tree event filters. This allows users to skip the call to `khepri_evf` and make it easier to use the API with a path directly.
dumbbell
force-pushed
the
revisit-public-api
branch
from
April 20, 2022 11:53
d3d4ac9
to
f871669
Compare
the-mikedavis
added a commit
to the-mikedavis/khepri-benchmark
that referenced
this pull request
Apr 24, 2022
The khepri API was refactored in rabbitmq/khepri#79. This commit makes a few changes to align khepri_benchmark_khepri's calls to khepri: - cluster functions are moved into the khepri_cluster module - khepri:add_member/2 -> khepri_cluster:add_member/2 - khepri:members/1 -> khepri_cluster:members/1 - get/put/delete have been moved into the khepri module from khepri_machine - the kpayload_data record wrapper is no longer necessary
dumbbell
added a commit
that referenced
this pull request
Apr 25, 2022
The module was renamed from `khepri_clustering` to `khepri_cluster` as part of the development of #79. The rename is not part of the commit history but this reference was left over.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
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.
Here are the main changes to refine the public API:
khepri
: the entry point for 99% of the public APINow,
khepri
is the main entry point for everything public. There is no longer a high-level API inkhepri
and a lower-level API inkhepri_machine
.khepri
provides more user-friendly functions and we will add more when we see fit.For instance, it's possible to get the data of an existing tree node directly:
Inside transactions,
khepri_tx
provides the same API askhepri
, except when the function does not make sense in the context of a transaction.khepri_machine
is now considered private and should not be called directly.Unix-like paths are first-class citizen
Wherever a function took a native path (
[stock, wood, <<"oak">>]
), it now also accepts a Unix-like path ("/:stock/:wood/oak"
). For instance, the following two lines are the same:Note that the Unix-like path syntax changed:
:atom
binary
Payload types and event filters are automatically detected and wrapped
The
#kpayload_*{}
are now private and payload types are automatically detected.Likewise for event filters: it is possible to specify a path where an event filter is accepted and it will be automatically considered a tree event filter with default properties. The
#kevf_*{}
records are also private now. For example:Note that again, Unix-like paths are accepted.