Skip to content
This repository has been archived by the owner on Jul 1, 2024. It is now read-only.

sdk: add evaluateBatch() #78

Merged
merged 3 commits into from
Jun 26, 2024
Merged

sdk: add evaluateBatch() #78

merged 3 commits into from
Jun 26, 2024

Conversation

srenatus
Copy link
Member

@srenatus srenatus commented Jun 18, 2024

TODO:

  • deal with mixed results
  • allow fall-back to a sequence of evaluate() calls
  • inject license via GHA secrets

tests/authorizer.test.ts Fixed Show fixed Hide fixed
@srenatus srenatus force-pushed the sr/batch-eval branch 12 times, most recently from 1bafe4b to b05b602 Compare June 21, 2024 09:11
tests/authorizer.test.ts Dismissed Show dismissed Hide dismissed
tests/authorizer.test.ts Dismissed Show dismissed Hide dismissed
@srenatus srenatus force-pushed the sr/batch-eval branch 2 times, most recently from b3e7951 to a64d1a5 Compare June 21, 2024 10:37
@srenatus srenatus marked this pull request as ready for review June 21, 2024 10:39
Copy link
Member Author

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

Some reviewer notes 👇

propertyName: http_status_code
mapping:
"200": "#/components/schemas/SuccessfulPolicyResponse"
"500": "#/components/schemas/ServerError"
Copy link
Member Author

Choose a reason for hiding this comment

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

ℹ️ We'll upstream this if it turns out to do the trick for C#, too!

Copy link
Member Author

Choose a reason for hiding this comment

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

...and drop the commit (StyraInc/opa-typescript@93fa195)

src/opaclient.ts Outdated Show resolved Hide resolved
@@ -11,7 +11,7 @@ import { OPAClient, ToInput, Input, Result } from "../src";
import { HTTPClient } from "../src/lib/http";
Copy link
Member Author

Choose a reason for hiding this comment

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

Restructured the tests a bit. These are the cases that are included now:

▶ tests
  ▶ with eopa
    ✔ can be called without types, without input (11.54083ms)
    ✔ rejects with server error on failure (2.418835ms)
    ✔ can be called with input==false (8.864047ms)
    ▶ default
      ✔ can be called without types, without input (5.759252ms)
      ✔ can be called with input (4.895409ms)
    ▶ default (11.259281ms)

    ✔ supports rules with slashes (3.708803ms)
    ✔ supports input/result types (4.165662ms)
    ✔ supports input of type bool (4.347747ms)
    ✔ calls stringify on a class as input (4.387289ms)
    ✔ supports input class implementing ToInput (4.131907ms)
    ✔ supports result class implementing FromResult (3.347218ms)
    ✔ allows custom low-level SDKOptions' HTTPClient (3.213485ms)
    ✔ allows fetch options (0.774398ms)
    ✔ allows custom headers (3.010292ms)
    ✔ supports rules with slashes when proxied (5.027933ms)
    ▶ batch
      ✔ supports rules with slashes (12.372602ms)
      ✔ can be called with input==false (5.318301ms)
      ✔ calls stringify on a class as input (4.528926ms)
      ✔ supports input class implementing ToInput (3.98718ms)
      ✔ supports result class implementing FromResult (3.364944ms)
      ✔ allows custom low-level SDKOptions' HTTPClient (3.036248ms)
      ✔ allows fetch options (1.398168ms)
      ✔ allows custom headers (3.596054ms)
      ✔ supports rules with slashes when proxied (3.562676ms)
      ✔ returns mixed-mode result on a failure (3.875064ms)
      ✔ rejects mixed-mode result if instructed (0.618417ms)
    ▶ batch (46.664602ms)

  ▶ with eopa (2963.970092ms)

  ▶ with opa
    ▶ batch-fallback
      ✔ fails unless instructed to fallback (0.910582ms)
      ▶ fallback
        ✔ returns same result, without errors (14.021788ms)
        ✔ returns same result, with errors (6.970417ms)
      ▶ fallback (21.21245ms)

      ✔ rejects mixed-mode result if instructed (0.599692ms)
    ▶ batch-fallback (148.2918ms)

  ▶ with opa (1460.965599ms)

▶ tests (5400.462863ms)

src/opaclient.ts Outdated Show resolved Hide resolved
Copy link
Member

@philipaconrad philipaconrad left a comment

Choose a reason for hiding this comment

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

Looks good to me!

One bit of feedback I'll add to what @charlesdaniels noted around how feature detection works for the Batch Query API: we potentially can memoize the feature detection for whether or not a remote "OPA" supports the Batch Query API or not, allowing future batch requests to skip the "feature detect" query in the future. That sort of feature can definitely wait until a future PR though!

src/opaclient.ts Outdated Show resolved Hide resolved
@srenatus srenatus force-pushed the sr/batch-eval branch 3 times, most recently from 00c2435 to 9d55282 Compare June 26, 2024 07:56
srenatus and others added 3 commits June 26, 2024 09:59
options for that method:

- rejectMixed (default false) -- if a mixed result (ie. at least one
  element of the batch failed evaluation) causes a promise rejection
- fallback (default false) -- if a 404 for the batch endpoint should
  degrade transparently into a sequence of single evaluations. This
  would be the case when talking to OPA (not Enterprise OPA).

Signed-off-by: Stephan Renatus <stephan@styra.com>
Signed-off-by: Stephan Renatus <stephan@styra.com>
@srenatus srenatus merged commit 0c22fd1 into main Jun 26, 2024
4 checks passed
@srenatus srenatus deleted the sr/batch-eval branch June 26, 2024 08:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants