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

sql: make session_revival_token.enabled tenant-ro #77493

Merged
merged 1 commit into from
Mar 9, 2022

Conversation

rafiss
Copy link
Collaborator

@rafiss rafiss commented Mar 8, 2022

I was hoping to wait for the new cluster setting syntax to be completed,
but since it's getting close to the branch cut time I'd rather merge
this now so we don't forget at the last minute.

Release justification: low risk change to new functionality.

Release note: None

@rafiss rafiss requested review from RichardJCai, jaylim-crl and a team March 8, 2022 15:54
@rafiss rafiss requested a review from a team as a code owner March 8, 2022 15:54
@rafiss rafiss removed the request for review from a team March 8, 2022 15:54
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rafiss rafiss force-pushed the session-revival-tenant-read-only branch from 85a71ae to c30c7b9 Compare March 8, 2022 16:29
Copy link
Collaborator

@jaylim-crl jaylim-crl left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @RichardJCai)

Copy link
Collaborator

@jaylim-crl jaylim-crl left a comment

Choose a reason for hiding this comment

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

Also, looks like the logic tests failed:

         pq: relation "system.tenant_settings" does not exist

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

I was hoping to wait for the new cluster setting syntax to be completed,
but since it's getting close to the branch cut time I'd rather merge
this now so we don't forget at the last minute.

Release justification: low risk change to new functionality.

Release note: None
@rafiss rafiss force-pushed the session-revival-tenant-read-only branch from c30c7b9 to b877524 Compare March 8, 2022 21:36
@rafiss
Copy link
Collaborator Author

rafiss commented Mar 8, 2022

tftr!

bors r=otan,jaylim-crl

@craig
Copy link
Contributor

craig bot commented Mar 9, 2022

Build succeeded:

@craig craig bot merged commit f15acd1 into cockroachdb:master Mar 9, 2022
@rafiss rafiss deleted the session-revival-tenant-read-only branch March 9, 2022 03:57
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.

4 participants