-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
scripts: add drtprod
wrapper for managing drt clusters
#120056
Conversation
The commit message could indicate why we need the wrapper instead of exporting the env vars locally; I also find |
I think it is likely that someone will forget one of the vars -- and we do have to pass both and maybe more to come -- or accidentally pass it when they do not mean to (e.g. one reason we want a separate project is to prevent
We put I'm fine moving it to |
Thanks for explaining. Re: location, the But it's pretty inconsequential so if I'm the only one of this opinion, it's fine. |
I've moved it to While I'm here, I've also expanded the script slightly to validate arguments match the requirements for DRT (secure mode and static ports), which I think helps show the value of the wrapper. |
Release note: none. Epic: none.
drtprod
wrapper for managing drt clusters
echo "--secure is required when starting DRT clusters" | ||
exit 1 | ||
fi | ||
if [[ "$*" != *"--sql-port 26257"* ]]; then |
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.
Thanks for adding this, I added some additional checks in roachprod
to prevent using DNS/services on clusters that do not support it. See #120141
Once that is merged the logic here will hopefully not be necessary anymore.
TFTR! bors r+ |
Build succeeded: |
Release note: none.
Epic: none.