Skip to content
This repository has been archived by the owner on Mar 29, 2024. It is now read-only.

Updated the proto support, fixed some perf issues and a few more things #11

Closed
wants to merge 17 commits into from
Closed

Updated the proto support, fixed some perf issues and a few more things #11

wants to merge 17 commits into from

Conversation

r3v2d0g
Copy link
Contributor

@r3v2d0g r3v2d0g commented Oct 2, 2019

Hey 👋!

Here's what I did in this PR:

  • The proto files were updated and a build.rs file to generate Rust code for them was added.
  • The requests and responses were updated to support newly added fields.
  • A few unwraps were removed to return Result instead.
  • Support for using Vec<u8> as keys and values was added.
  • Clients are now all Cloneable and requests and responses (and their related types) now are Debugable and Cloneable.
  • A KeepAlive stream has been added. It is returned by one of two new methods of LeaseClient: keep_alive and keep_alive_at_interval (keep_alive_once has been removed).

Closes #6

…E perf"s and a few more things

- The proto files and their dependencies are located in the `proto` directory.
- The `build.rs` will generate Rust files exposing the APIs in the `src/proto` directory.
- The `KeyValue`, `ResponseHeader` and `kv`'s `*Response` structs now only contain the data they are exposing.
- `kv`'s `*Request` structs now store `Vec<u8>`s instead of `String`s.
- `kv`'s `*Response` structs' methods now return references where cloning was used before.
- The content of the `prefix` methods has been fixed (when providing it a key whose last char(s) is `== 255`, those chars were removed from the prefix, thus possibly making the response contain keys that didn't contain the user defined prefix).
- `KeyValue` now exposes a `raw_key` and `raw_value` methods and its `key` and `value` now return a `Result` instead of unwrapping.
- `PutResponse`'s `prev_kv` method now returns `None` in case it wasn't part of the server's response.
- `TxnRequest` now has a new method `when_lease` and its `when_mod_revision` has been fixed (it was defining the compare target as `CREATE` instead of `MOD`).
@r3v2d0g r3v2d0g changed the title [WIP] Update the proto support, fixed some perf issues and a few more things [WIP] Updated the proto support, fixed some perf issues and a few more things Oct 2, 2019
# Conflicts:
#	src/kv/get.rs
#	src/proto/auth.rs
#	src/proto/kv.rs
#	src/proto/lock.rs
#	src/proto/lock_grpc.rs
#	src/proto/rpc.rs
#	src/proto/rpc_grpc.rs
…t) and (min|max)_(mod|create)_revision to GetRequest

- Fixed some formatting issues (mainly replaced the tabs that I used in the my previous commit with spaces).
- Finished replacing `String`s with `Vec<u8>` in the `kv` module.
- Added support for `sort_order` and `sort_target` in `GetRequest`.
- Added support for `(min|max)_mod_revision` and `(min|max)_create_revision` in `GetRequest`.
- Reordered a few methods and fields in the `kv` module to have the same order as defined in the `proto` files.
- Updated `lease`'s `*Response` structs to only contain the fields they are exposing.
- Updated `lease`'s `*Response` to return references wherever possible.
- Updated the `keys` method of `TtlResponse` to return an `Err` if it can't convert the key to UTF-8.
- Added a `raw_keys` method to `TtlResponse`.
- Added support for `id` in `GrantRequest`.
- Updated `lock`'s `*Response` structs to only contain the fields they are exposing.
- Updated `lock`'s `*Response` to return references wherever possible.
- Updated `LockRequest` to accept and store `Vec<u8>`s instead of `String`s.
- Updated the `key` method of `LockResponse` to return an `Err` if it can't convert the key to UTF-8.
- Added a `raw_key` method to `LockResponse`.
- Updated `WatchResponse` to only contain the fields they are exposing.
- Updated `WatchResponse` to return references wherever possible.
- Updated `WatchRequest` to accept and store `Vec<u8>`s instead of `String`s.
- Added a `range` method to `WatchRequest`.
- Added support for `filters`, `watch_id` and `fragment` in `WatchRequest`.
- Added support for `cancel_reason` and `fragment` in `WatchResponse`.
- Updated `Watch` to not unwrap when `send.poll_complete()` fails.
- `Watch` and `KeepAlive` are and should only be used by the crate and are thus declared as such.
- `prelude` was exporting `KvClient` and `LeaseClient` but not the other clients, so they were removed.
…xed formatting

- Added a `keep_alive_at_interval` to `LeaseClient`.
- After receiving a `KeepAliveResponse`, `KeepAlive` will wait for the provided interval before sending a new request.
@r3v2d0g r3v2d0g marked this pull request as ready for review October 3, 2019 20:00
@r3v2d0g r3v2d0g changed the title [WIP] Updated the proto support, fixed some perf issues and a few more things Updated the proto support, fixed some perf issues and a few more things Oct 3, 2019
@r3v2d0g
Copy link
Contributor Author

r3v2d0g commented Oct 10, 2019

Hey 👋!

So, I think this PR is ready for a review 😃

NOTE: I documented what I did in each commit in their description message.
NOTE: I'll finish removing all unwraps in an other PR where I'll also migrate the library to std's futures.

@zarvd
Copy link
Owner

zarvd commented Oct 24, 2019

Hi, thanks for all the contributions you have done. 😄

I think it does not need to import gogo and some other thing which does not affect in Rust proto compiler. The API updates like remove unwrap and return result and support low-level interactive are great, but the backward-compatibility is broken.

As async/await will be available in the 1.39 release and tonic emerges, I think it's time to embrace the new thing in 0.2 version.

And for the 0.1 etcd-rs, I prefer not to release breaking changes. Correct me if I am wrong, thanks.

@r3v2d0g
Copy link
Contributor Author

r3v2d0g commented Oct 24, 2019

Hi, thanks for all the contributions you have done.

Hey 👋! 😄

I think it does not need to import gogo and some other thing which does not affect in Rust proto compiler.

Right, I needed to because I wanted to use etcd's proto files as-is 😃

The API updates like remove unwrap and return result and support low-level interactive are great, but the backward-compatibility is broken.

As async/await will be available in the 1.39 release and tonic emerges, I think it's time to embrace the new thing in 0.2 version.

And for the 0.1 etcd-rs, I prefer not to release breaking changes. Correct me if I am wrong, thanks.

It makes complete sense to me! I would be glad to update the library to tonic and async/await (in which case, should I do so in this PR or would you like me to open a new one?), unless you would prefer to do it yourself.

Cheers!

@zarvd
Copy link
Owner

zarvd commented Nov 15, 2019

@azastrael master branch supports async/await and tonic now. Thanks again for this PR.

@zarvd zarvd closed this Nov 15, 2019
@r3v2d0g r3v2d0g deleted the proto-update-and-perfs branch November 15, 2020 12:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return references instead of clones
2 participants