Skip to content

Conversation

@yuzefovich
Copy link
Member

@yuzefovich yuzefovich commented Nov 6, 2025

upgrade: remove unused SQLStats testing knobs

sql: do not set sqlstats knobs

This was added in 2fad679 because we called CreateTestServerParams in other tests. That is no longer the case as of 5adea01, so I don't think we need that.

sql: remove createTestServerParamsAllowTenants

This commit removes createTestServerParamsAllowTenants. Previously, this method returned two arguments, but the second one was only used only in a handful of spots - we now inline the necessary code to create CommandFilters in each spot. In all other spots the helper method no longer (given the change in the previous commit) provided any value, so we replaced all its usages with directly constructing the TestServerArgs.

Additionally some stale comments are removed.

Epic: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@yuzefovich yuzefovich added the O-No-AI-Review Prevents AI Review from running label Nov 6, 2025
@yuzefovich yuzefovich force-pushed the tenant-cleanup branch 3 times, most recently from 216fabf to 69524ea Compare November 7, 2025 21:17
@yuzefovich yuzefovich changed the title WIP on minor test cleanup sql: remove createTestServerParamsAllowTenants Nov 7, 2025
@yuzefovich yuzefovich marked this pull request as ready for review November 7, 2025 21:19
@yuzefovich yuzefovich requested review from a team as code owners November 7, 2025 21:19
@yuzefovich yuzefovich removed the O-No-AI-Review Prevents AI Review from running label Nov 7, 2025
@cockroachdb cockroachdb deleted a comment from blathers-crl bot Nov 7, 2025
Copy link
Contributor

@shubhamdhama shubhamdhama left a comment

Choose a reason for hiding this comment

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

🔥

This was added in 2fad679 because we
called CreateTestServerParams in other tests. That is no longer the case
as of 5adea01, so I don't think we need
that.

Release note: None
This commit removes `createTestServerParamsAllowTenants`. Previously,
this method returned two arguments, but the second one was only used
only in a handful of spots - we now inline the necessary code to create
CommandFilters in each spot. In all other spots the helper method no
longer (given the change in the previous commit) provided any value, so
we replaced all its usages with directly constructing the
TestServerArgs.

Additionally some stale comments are removed.

Release note: None
Copy link
Member Author

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

TFTR!

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@craig
Copy link
Contributor

craig bot commented Nov 12, 2025

@craig craig bot merged commit daae10b into cockroachdb:master Nov 12, 2025
21 of 23 checks passed
@yuzefovich yuzefovich deleted the tenant-cleanup branch November 12, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants