fix: better error message for ./risedev d timeout
#18756
Conversation
For the reason when use `./risedev d` failed to start, when the server and port is in used the error only shows `failed to wait for closing` This patch make the message better. Also make docker kill the containers named risingwave(which started by docker-compose. Signed-off-by: yihong0618 <[email protected]>
| @@ -713,7 +713,7 @@ ${TMUX} list-windows -F "#{window_name} #{pane_id}" \ | |||
| | xargs -I {} ${TMUX} send-keys -t {} C-c | |||
|
|
|||
| # Stop docker components | |||
| containers=$(docker ps -a -q -f name=risedev- 2>/dev/null) || true | |||
| containers=$(docker ps -q -f name='risedev|risingwave'- 2>/dev/null) || true | |||
There was a problem hiding this comment.
The risedev k command is only supposed to kill services started by risedev 🤔. But I don't mind too much it killing other risingwaves anyway.
There was a problem hiding this comment.
I find it a bit weird though... since running two instances does not imply they are working on the same ports.
What if the developer intends to run one production instance with docker and one development instance with risedev together, for example, to compare their behaviors?
There was a problem hiding this comment.
I guess a better way is to show the name of process which uses the port we need, and let the user to decide what to do next.
There was a problem hiding this comment.
What if
I don't like to talk about imaginary problems, and I don't have preferences on this specific issue. 😄
I guess a better way is to show the name of process which uses the port we need, and let the user to decide what to do next.
elegant!
e6435e6
For the reason when use
./risedev dfailed to start, when the server and port is in used the error only showsfailed to wait for closingThis patch make the message better. Also make docker kill the containers named risingwave(which started by docker-compose.when start docker-compose
then forget to close the docker container by
compose down./risedev d error message only
failed to wait for closingthis path make the error message betteralso fix the
./risedev konly kill the container namedrisedevthis patch addrisingwavename to./risedev kto make it easy to useI hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.