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

distsqlpb: rename protos back to distsqlrun #36572

Merged
merged 1 commit into from
Apr 5, 2019

Conversation

jordanlewis
Copy link
Member

The move of code to the distsqlpb package also inadvertently renamed
the actual protobuf service endpoints and messages, which represented an
accidental breaking change between 2.1 and 19.1 and would result in
errors in mixed clusters.

This commit reverts that change, restoring harmony for mixed-version
clusters.

This needs a backport and we can't release without that backport.

Touches #36565.

Release note: None

@jordanlewis jordanlewis requested review from tbg, asubiotto and a team April 5, 2019 14:18
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@tbg tbg left a comment

Choose a reason for hiding this comment

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

LGTM though you may want to add a comment because it'll look inviting for someone passing by to "harmonize" the package and directory name again.

Copy link
Contributor

@asubiotto asubiotto left a comment

Choose a reason for hiding this comment

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

LGTM. I assume you tested with #36565? I wonder if we could also add a service name unit-test to avoid having to rely on that.

The move of code to the `distsqlpb` package also inadvertently renamed
the actual protobuf service endpoints and messages, which represented an
accidental breaking change between 2.1 and 19.1 and would result in
errors in mixed clusters.

This commit reverts that change, restoring harmony for mixed-version
clusters.

Release note: None
@jordanlewis
Copy link
Member Author

Added a comment. I did manual testing to ensure that a mixed-version cluster still works - I think this will be caught by @tbg's roachtest.

TFTR!

bors r+

@jordanlewis
Copy link
Member Author

Notice me, bors!

bors r+

craig bot pushed a commit that referenced this pull request Apr 5, 2019
36572: distsqlpb: rename protos back to distsqlrun r=jordanlewis a=jordanlewis

The move of code to the `distsqlpb` package also inadvertently renamed
the actual protobuf service endpoints and messages, which represented an
accidental breaking change between 2.1 and 19.1 and would result in
errors in mixed clusters.

This commit reverts that change, restoring harmony for mixed-version
clusters.

This needs a backport and we can't release without that backport.

Touches #36565.

Release note: None

Co-authored-by: Jordan Lewis <[email protected]>
@craig
Copy link
Contributor

craig bot commented Apr 5, 2019

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.

4 participants