-
Notifications
You must be signed in to change notification settings - Fork 0
proto: support queue #9
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
base: master
Are you sure you want to change the base?
Conversation
e61b13b to
b4137d8
Compare
aeon_queue.proto
Outdated
| string consumer = 2; | ||
| // Max number of returned messages. | ||
| uint64 limit = 3; | ||
| // Time to live of the request. |
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.
This is TTL of the consumer, not of the request.
Actually, all the comments are quite confusing. Please revise - write a few words so that it's clear what each is one is used for and what it should be set to.
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.
Done. I've rewritten most of the comments and deleted the unnecessary ones.
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.
Still not good enough. We will ship this proto file to end users and it'd be nice if they could figure out how to use the API by just reading the comments. Try to pretend that you are a user unfamiliar with the implementation and write comments so that you can understand the purpose of each API function by just looking at them.
This patch adds the `push` field to `ExecuteRequest` and introduces `aeon_queue.proto` module. Needed for tarantool/aeon#488
b4137d8 to
21875fc
Compare
| // Time to live of the message. | ||
| double ttl = 3; | ||
| } | ||
| // Push function. |
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.
What arguments does this function take? In what language is it written? What is it supposed to return? Please write comments so that a user who isn't familiar with the implementation can figure out how to use this API.
| message TakeMessagesRequest { | ||
| // Topic name. | ||
| string topic = 1; | ||
| // Unique consumer name. |
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.
What's it used for?
| // Takes messages from a shard-local queue. | ||
| rpc TakeMessages(TakeMessagesRequest) returns (TakeMessagesResponse) {} | ||
|
|
||
| // Releases messages. |
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.
What happens if they aren't released?
This patch adds the
pushfield toExecuteRequestand introducesaeon_queue.protomodule.Needed for tarantool/aeon#488