deps: Bump proxy-wasm-cpp-host #42600
Conversation
- Most of the patches are no longer needed, were applied upstream - Upstream added `virtual` to `BaseContext::wasm()`, forcing us to rename ours to `wasmEnvoy()`, hence the big number of changes. Signed-off-by: Jonh Wendell <jwendell@redhat.com>
repokitteh-read-only
bot
added
the
deps
|
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to |
|
i forget the details but i think this one wants to be updated with wasmtime - iirc to the one set upstream |
Even if all tests pass? I wanted to update one dep at a time where possible. |
Signed-off-by: Jonh Wendell <jwendell@redhat.com>
|
yeah - i dont rem what the exact reason is - other than they need to be compatible - it might well be that our wasmtime version is actually ignored or similar i think its fine to update separately - but then i think update wasmtime (if needed) asap in that case |
|
@phlax this version of proxy-wasm-cpp-host still uses the same wasmtime we currently do: https://github.com/proxy-wasm/proxy-wasm-cpp-host/blob/c8868da77499d414f3dcd07e4fbe584dc9a1c30d/bazel/repositories.bzl#L314 There's a PR updating to the latest but it's not merged yet. So, we're safe :) |
| DefaultAllowOnHeadersStopIteration); | ||
| } | ||
|
|
||
| Wasm* Context::wasm() const { return static_cast<Wasm*>(wasm_); } |
There was a problem hiding this comment.
Why not just mark it as override?
There was a problem hiding this comment.
error: return type of virtual function 'wasm' is not covariant with the return type of the function it overrides ('Wasm' is incomplete)
Believe me, I tried :)
There was a problem hiding this comment.
This is weird. I think it was patched with virtual because of envoy but if envoy can't use it, there's no point in that virtual :)
There was a problem hiding this comment.
@rachgreen33 - do you have some build of Envoy with it? what's the intended use of virtual?
| virtual proxy_wasm::LogLevel getLogLevel() = 0; | ||
| virtual void error(std::string_view message) = 0; | ||
| // Allow integrations to handle specific FailStates differently. | ||
| - virtual void error(FailState fail_state, std::string_view message) { error(message); } |
There was a problem hiding this comment.
Simpler thing is to just erase the variable name: error(FailState, std::string_view)
|
@kyessenov @rachgreen33 Here's a much cleaner alternative, patching upstream to remove |
|
Closing in favor of #42634 |
Pull request was closed
4 participants
virtualtoBaseContext::wasm(), forcing us to rename ours towasmEnvoy(), hence the big number of changes.