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/zip: emit SQL table data using TSV by default #107474

Merged
merged 1 commit into from Jul 27, 2023

Conversation

knz
Copy link
Contributor

@knz knz commented Jul 24, 2023

Fixes #107473.
Epic: CRDB-28893

This is a partial revert of 35738d4.

It changes the default value of the --format flag back from JSON to TSV.

Release note (backward-incompatible change):
THIS RELEASE NOTE CANCELS THE CORRESPONDING PREVIOUS BACKWARD-INCOMPATIBLE CHANGE. New behavior, compatible with previous versions of CockroachDB: the command cockroach debug zip stores data retrieved from SQL tables in the remote cluster using the TSV format by default.

Release note (cli change): The default value of the --format parameter to cockroach debug zip is tsv, like other CLI commands that can extract SQL data.

@blathers-crl
Copy link

blathers-crl bot commented Jul 24, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@knz knz requested a review from abarganier July 24, 2023 18:54
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz
Copy link
Contributor Author

knz commented Jul 24, 2023

i'll wait for CI to tell me which tests need updating.

@knz knz marked this pull request as ready for review July 25, 2023 10:49
@knz knz requested review from a team as code owners July 25, 2023 10:49
Copy link
Member

@abarganier abarganier left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @knz)


pkg/cli/testdata/zip/testzip_external_process_virtualization line 155 at r1 (raw file):

[node ?] ? log files found
[node 1] [log file: ...
[node 1] [log file: ...

nit: Just curious - what about this change impacted the test outputs such that we have these extra lines?

This is a partial revert of 35738d4.

It changes the default value of the `--format` flag back from JSON to
TSV.

Release note (backward-incompatible change):
THIS RELEASE NOTE CANCELS THE CORRESPONDING PREVIOUS BACKWARD-INCOMPATIBLE CHANGE.
New behavior, compatible with previous versions of CockroachDB: the
command `cockroach debug zip` stores data retrieved from SQL tables in
the remote cluster using the TSV format by default.

Release note (cli change): The default value of the `--format`
parameter to `cockroach debug zip` is `tsv`, like other CLI commands
that can extract SQL data.
Copy link
Contributor Author

@knz knz left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @abarganier)


pkg/cli/testdata/zip/testzip_external_process_virtualization line 155 at r1 (raw file):

Previously, abarganier (Alex Barganier) wrote…

nit: Just curious - what about this change impacted the test outputs such that we have these extra lines?

This was actually some extra non-determinism we had there already (and thus possible source of flakes).

Fixed.

@knz
Copy link
Contributor Author

knz commented Jul 27, 2023

bors r=abarganier

@craig craig bot merged commit 68e43c8 into cockroachdb:master Jul 27, 2023
7 checks passed
@craig
Copy link
Contributor

craig bot commented Jul 27, 2023

Build succeeded:

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.

cli/zip: restore the previous default of --format to tsv
3 participants