Repository navigation
gh-123 Add task state diagram - #145
Conversation
LeonidVas
left a comment
LeonidVas
left a comment
There was a problem hiding this comment.
Hi! Thank you for the patchset.
I experimented with the diagram and it looks like I was able to achieve a nicer picture.
@startuml
[*] -down-> READY : put()
[*] -down-> DELAYED : put('my_task_data',\n{ delay = delay })
READY -down-> TAKEN : take()
READY -down-> DONE : delete() /\nttl timeout
READY -down-> BURIED : bury()
TAKEN -up-> READY : release() /\nttr timeout
TAKEN -left-> DELAYED : release\n(id, { delay = delay })
TAKEN -down-> DONE : ack() / delete()
TAKEN -right-> BURIED : bury()
BURIED -down-> DONE : delete() /\nttl timeout
BURIED -up-> READY : kick()
DELAYED -up-> READY : timeout
DELAYED -down-> DONE : delete()
DONE -down-> [*] : remove executed task
@enduml
I suggest adding images to the doc/images with the plantUML source for future editing.
| The request "require 'queue'" causes automatic creation of | ||
| the `_queue` space, unless it already exists. The same request | ||
| also sets the necessary space triggers and other objects | ||
| associated with queues. |
There was a problem hiding this comment.
Why did you decide to remove the double space ("\n" in md) here and below?
These are unnecessary changes. If you want to update something that not related with patchset, create a separate PR.
I suggest you read https://github.com/tarantool/tarantool/wiki/Code-review-procedure#general-coding-points-to-check and https://github.com/tarantool/tarantool/wiki/Code-review-procedure#commit-message .
There was a problem hiding this comment.
I got your point on including only relevant updates into PR.
If these double spaces serve some purpose and are not just typos, I will correct this.
There was a problem hiding this comment.
Two whitespaces at the end of line issues a <br /> tag into HTML. Personally I consider this feature as broken in two ways: the syntax is weird, the feature is not needed (it always better to just split text with paragraphs or use three-backticks code blocks). So I prefer to don't use it. And GFM (GitHub's Markdown dialect) anyway preserves line breaks, so it is useless on GitHub.
Anyway, Leonid is right, it is not relevant to the problem that you are solving.
There was a problem hiding this comment.
I got your point on including only relevant updates into PR.
If these double spaces serve some purpose and are not just typos, I will correct this.
Yep. These are not typos.
d42f032 to
fc97cc2
Compare
LeonidVas
left a comment
LeonidVas
left a comment
There was a problem hiding this comment.
Hi!
I'll comfortable with one of #145 (comment) , #145 (comment) .
Please correct the code according to the comments, squash the commits, self-review the code and update the PR.
| The request "require 'queue'" causes automatic creation of | ||
| the `_queue` space, unless it already exists. The same request | ||
| also sets the necessary space triggers and other objects | ||
| associated with queues. |
There was a problem hiding this comment.
I got your point on including only relevant updates into PR.
If these double spaces serve some purpose and are not just typos, I will correct this.
Yep. These are not typos.
LeonidVas
left a comment
LeonidVas
left a comment
There was a problem hiding this comment.
Mistakenly chose "Approve" instead of "Request changes" in #145 (review)
002727d to
6c48d16
Compare
Done |
The diagram gives a concise and holistic view of all the task states and what method triggers each transition between the states. Closes #123
6c48d16 to
cdd2d71
Compare
|
Does not look well in the dark mode.
|
Let's do it beautifully in the future #148 |
4 participants
No description provided.