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

Implement Support for Batch Evaluation #73

Merged
merged 7 commits into from
Aug 19, 2024
Merged

Conversation

charlesdaniels
Copy link
Member

@charlesdaniels charlesdaniels commented Aug 13, 2024

This PR adds support for Enterprise OPA's batch evaluation API, including automatic fallback to separate, serial requests when the batch evaluation API is not available. Detection of batch evaluation API support is cached on a per-client basis to avoid repeated attempts to request a nonexistant API endpoint.

This also introduces a new return type, OPAResult, defers final output type conversion and exception throwing until it's .get() method is called. This allows for mixed success and failure in batch evaluation to be handled gracefully, without losing the outputs from requests which did succeed. A version of the existing, scalar API support has been added to allow OPAResult to also be used in non-batch use cases.

Before this PR can be merged, te changes to the OpanAPI spec made to support it have to be merged upstream (#90).

Previous work was in the charles/batch-evaluate branch, but the
extensive merge conflicts in the SE-generated code made it difficult to
rebase onto main. I have manually ported over the changes into this new
branch based on main, and have the tests passing again.
Batch evaluation seems to be working with the experimental change I made
to the OAS.

# We name the eopa binary opa so that the entrypoint script does not need to be
# modified.
COPY --from=ghcr.io/styrainc/enterprise-opa:latest /ko-app/enterprise-opa-private /usr/bin/opa
Copy link
Member

Choose a reason for hiding this comment

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

ℹ️ with the next release, this should no longer be necessary. In the mean time, you could use :edge here

Copy link
Member Author

Choose a reason for hiding this comment

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

The binary still needs to be copied into the container one way or the other. We can't directly use the eopa container, because we need nginx inside it as well.

@charlesdaniels charlesdaniels marked this pull request as ready for review August 19, 2024 20:08
@charlesdaniels charlesdaniels merged commit 609a79b into main Aug 19, 2024
4 checks passed
@charlesdaniels charlesdaniels deleted the charles/batch-eval-2 branch August 19, 2024 20:09
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.

2 participants