plugin: allow selecting wasm target for evaluation #276
Conversation
srenatus
force-pushed
the
sr/wasm/opt-in
branch
3 times, most recently
from
61c6e3d to
4fb6822
Compare
srenatus
force-pushed
the
sr/wasm/opt-in
branch
from
4fb6822 to
ac0d67f
Compare
This turns the simple grpc tests scenario into a matrix test for both evaluation variants. Fixes open-policy-agent/opa#3716. Signed-off-by: Stephan Renatus <[email protected]>
srenatus
force-pushed
the
sr/wasm/opt-in
branch
from
ac0d67f to
62e3947
Compare
| target := "rego" | ||
| if ec, ok := evalContext.(EvalContextWithTarget); ok { | ||
| target = ec.Target() | ||
| } |
There was a problem hiding this comment.
This feels like a better approach. This also means that if we wanted to add something new, we would create a new interface ? But maybe there's no avoiding that 🤷♂️
There was a problem hiding this comment.
We can have anonymous interfaces, we don't need to name it. We could do
if ec, ok := evalContext.(interface{ Target() string }); ok {
target = ec.Target()
}However, that'll make the assertion bit you've pointed at in the other comment impossible, I think. We'd need to find a way to get a similar assurance somehow, like with an extra test of some sort 🤔
There was a problem hiding this comment.
LGTM. We should make sure we include this new config field in the docs.
|
@srenatus do we want to revive this PR ? |
|
Let's do it when someone asks for it 😄 what do you think? |
|
OTOH it would perhaps be good if this didn't blow up when fed a wasm bundle... We should perhaps test that? |
|
It probably doesn't, the dependency should be there, I've just never tried. |
This turns the simple grpc tests scenario into a matrix test for both
evaluation variants.
Fixes #650.