Skip to content
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

cli: avoid deprecation warnings for deprecated flags #29446

Merged
merged 1 commit into from
Aug 31, 2018

Conversation

knz
Copy link
Contributor

@knz knz commented Aug 31, 2018

Suggested/recommended by @a-robinson.

The flags --host, --advertise-host, etc are now deprecated, but
there is no cost in continuing to support them. Also users migrating
from previous versions are not losing anything (or missing out) by
continuing to use them. Forcing a warning to appear when they are used
does not bring any tangible benefit and risks creating operator
fatigue.

So this patch removes the warning (but keeps the deprecated flags
hidden, so that new users are guided to the new flags).

Release note: None

@knz knz requested a review from a-robinson August 31, 2018 21:11
@knz knz requested a review from a team as a code owner August 31, 2018 21:11
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Suggested/recommended by @a-robinson.

The flags `--host`, `--advertise-host`, etc are now deprecated, but
there is no cost in continuing to support them. Also users migrating
from previous versions are not losing anything (or missing out) by
continuing to use them. Forcing a warning to appear when they are used
does not bring any tangible benefit and risks creating operator
fatigue.

So this patch removes the warning (but keeps the deprecated flags
hidden, so that new users are guided to the new flags).

Release note: None
@knz knz force-pushed the 20180831-cli-undeprecate branch from 6d2f475 to 5a41e06 Compare August 31, 2018 21:13
Copy link
Contributor

@a-robinson a-robinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@knz
Copy link
Contributor Author

knz commented Aug 31, 2018

bors r+

craig bot pushed a commit that referenced this pull request Aug 31, 2018
28688: cli: periodically flush csv/tsv output r=knz a=knz

Fixes #28654.

The "sinkless" version of changefeeds continuously streams back
results to the user over pgwire. Prior to this patch, this data could
not be consumed effectively with `cockroach sql` using the tsv/csv
output, because the tsv/csv formatter buffers rows internally.

This patch makes tsv/csv output in `cockroach sql` an effective way to
consume changefeeds by ensuring an upper bound on the time rows stays
buffered inside the formatter. The flush period is fixed to 5 seconds.

For context, all the other formatters except for `table` are
line-buffered and thus flush on every row. `table` is a world of its
own which buffers *all* the rows until the query is complete, and that
is unlikely to change any time soon, so this patch doesn't touch that
either.

Release note (cli change): The `csv` and `tsv` formats for `cockroach`
commands that output result rows now buffer data for a maximum of 5
seconds. This makes it possible to e.g. view SQL changefeeds
interactively with `cockroach sql` and `cockroach demo`.

29446: cli: avoid deprecation warnings for deprecated flags r=knz a=knz

Suggested/recommended by @a-robinson.

The flags `--host`, `--advertise-host`, etc are now deprecated, but
there is no cost in continuing to support them. Also users migrating
from previous versions are not losing anything (or missing out) by
continuing to use them. Forcing a warning to appear when they are used
does not bring any tangible benefit and risks creating operator
fatigue.

So this patch removes the warning (but keeps the deprecated flags
hidden, so that new users are guided to the new flags).

Release note: None

Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
@craig
Copy link
Contributor

craig bot commented Aug 31, 2018

Build succeeded

craig bot pushed a commit that referenced this pull request Aug 31, 2018
29448: release-2.1: cli: avoid deprecation warnings for deprecated flags r=knz a=knz

Backport 1/1 commits from #29446.

/cc @cockroachdb/release

---

Suggested/recommended by @a-robinson.

The flags `--host`, `--advertise-host`, etc are now deprecated, but
there is no cost in continuing to support them. Also users migrating
from previous versions are not losing anything (or missing out) by
continuing to use them. Forcing a warning to appear when they are used
does not bring any tangible benefit and risks creating operator
fatigue.

So this patch removes the warning (but keeps the deprecated flags
hidden, so that new users are guided to the new flags).

Release note: None


Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
@craig craig bot merged commit 5a41e06 into cockroachdb:master Aug 31, 2018
craig bot pushed a commit that referenced this pull request Sep 1, 2018
29448: release-2.1: cli: avoid deprecation warnings for deprecated flags r=knz a=knz

Backport 1/1 commits from #29446.

/cc @cockroachdb/release

---

Suggested/recommended by @a-robinson.

The flags `--host`, `--advertise-host`, etc are now deprecated, but
there is no cost in continuing to support them. Also users migrating
from previous versions are not losing anything (or missing out) by
continuing to use them. Forcing a warning to appear when they are used
does not bring any tangible benefit and risks creating operator
fatigue.

So this patch removes the warning (but keeps the deprecated flags
hidden, so that new users are guided to the new flags).

Release note: None


Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
@knz knz deleted the 20180831-cli-undeprecate branch February 14, 2019 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants