Revert "[native]Add data cache related server operations"#23157
Revert "[native]Add data cache related server operations"#23157majetideepak wants to merge 1 commit intoprestodb:masterfrom
Conversation
This reverts commit b73ab7d.
xiaoxmeng
left a comment
xiaoxmeng
left a comment
There was a problem hiding this comment.
@majetideepak do you know what's the failure of the test? I think the server operation change is unrelated to the failed test. I am on vocation but you can also ask @zacw7 to help fix this. Thanks!
zacw7
left a comment
zacw7
left a comment
There was a problem hiding this comment.
This change itself doesn't have issue. It just includes a breaking change in Velox: facebookincubator/velox#10261
|
@zacw7 @xiaoxmeng unfortunately, this commit included both an update to Velox and the server operations endpoint. So to avoid keeping trunk in a broken state, let's revert, then re-add without the Velox bump. @yingsu00 please note the above, can you please address? Thanks. |
@tdcmeehan since we already know the issue, why not make a forward fix from Velox side? It doesn't make sense to me to continue this revert. |
|
This change and the advanced Velox version are already being released to prod within Meta. Instead of reverting a whole bunch of changes together, is it possible to revert the one breaking change in Velox if we really want to fix the broken trunk ASAP to minimize the disruption. |
|
It's fine to revert in Velox first, then advance Velox here. But I have a question @zacw7: if we were to revert this PR here, are you suggesting this might affect your production deployment? |
|
I am closing this in favor of advancing Velox. |
It doesn't affect deployment directly. But this is already being rolled out to prod so we would prefer not have it reverted. |
4 participants
This reverts commit b73ab7d.
Fix deterministic CI failure.
Resolves: #23156