Skip to content

serverutils: redirect the implicit ApplicationLayerInterface properly#110001

Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom
knz:20230905-redirect-applayer
Sep 11, 2023
Merged

serverutils: redirect the implicit ApplicationLayerInterface properly#110001
craig[bot] merged 2 commits intocockroachdb:masterfrom
knz:20230905-redirect-applayer

Conversation

@knz
Copy link
Copy Markdown
Contributor

@knz knz commented Sep 5, 2023

All commits but the last 3 are from #110008.
Epic: CRDB-18499

Prior to this patch, the implicit ApplicationLayerInterface
implementation inside TestServerInterface was redirecting to
SystemLayer(). This was confusing, likely the source of bugs, and
resulted in insufficient test coverage.

This commit fixes it.

followup issues identified thanks to this work:

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@knz knz force-pushed the 20230905-redirect-applayer branch from a2e6173 to 08dc3d4 Compare September 5, 2023 14:12
@knz knz marked this pull request as ready for review September 5, 2023 14:19
@knz knz requested a review from a team September 5, 2023 14:19
@knz knz requested a review from a team as a code owner September 5, 2023 14:19
@knz knz requested a review from a team September 5, 2023 14:19
@knz knz requested review from a team as code owners September 5, 2023 14:19
@knz knz removed the request for review from a team September 5, 2023 14:19
@knz knz requested review from DrewKimball, Santamaura and srosenberg and removed request for a team September 5, 2023 14:19
@knz knz force-pushed the 20230905-redirect-applayer branch from 08dc3d4 to 8c1a502 Compare September 5, 2023 15:13
@knz
Copy link
Copy Markdown
Contributor Author

knz commented Sep 5, 2023

The remaining failure in //pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl is not specific to this PR and discussed here.

@knz knz force-pushed the 20230905-redirect-applayer branch from 8c1a502 to 9c663c6 Compare September 7, 2023 13:46
@yuzefovich
Copy link
Copy Markdown
Member

Nice, very happy to see this! LGTM, but I'll defer to others for approval.

@knz knz force-pushed the 20230905-redirect-applayer branch from 9c663c6 to 073cf38 Compare September 8, 2023 14:27
craig bot pushed a commit that referenced this pull request Sep 11, 2023
110008: *: improve some tests r=stevendanna,yuzefovich,abargainier a=knz

This PR contains the test improvements prerequisite to #110001.
Epic: CRDB-18499



Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
knz added 2 commits September 11, 2023 03:34
Prior to this patch, the *implicit* `ApplicationLayerInterface`
implementation inside `TestServerInterface` was redirecting to
`SystemLayer()`. This was confusing, likely the source of bugs, and
resulted in insufficient test coverage.

This commit fixes it.

Release note: None
@knz knz force-pushed the 20230905-redirect-applayer branch from 073cf38 to e648b81 Compare September 11, 2023 01:35
@knz
Copy link
Copy Markdown
Contributor Author

knz commented Sep 11, 2023

rebased; PTAL

Copy link
Copy Markdown
Collaborator

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

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

Reminder to remove the wip commit before merge.

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Sep 11, 2023

Reminder to remove the wip commit before merge.

Done.

@knz knz force-pushed the 20230905-redirect-applayer branch from e648b81 to 46009af Compare September 11, 2023 11:09
@knz
Copy link
Copy Markdown
Contributor Author

knz commented Sep 11, 2023

TFYR!

bors r=stevendanna

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 11, 2023

Build succeeded:

@craig craig bot merged commit dc0aaba into cockroachdb:master Sep 11, 2023
@knz knz deleted the 20230905-redirect-applayer branch September 11, 2023 12:07
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