feat: expose the interpreter via the various APIs #1241
Conversation
dhess
force-pushed
the
dhess/interp-api
branch
7 times, most recently
from
5e7f6c6
to
d7f8aa6
Compare
dhess
force-pushed
the
dhess/interp-api
branch
2 times, most recently
from
ed1d2db
to
25861cd
Compare
dhess
changed the title
dhess
force-pushed
the
dhess/interp-api
branch
5 times, most recently
from
2016c8a
to
047762d
Compare
This commit only adds these handlers at the `Primer.App` module level. Hooking the interpreter up to the API and HTTP service will come in subsequent commits. Note that we add two handlers, one for time-bounded requests, and one for unbounded requests. We will not expose the unbounded handler via the HTTP API, since that would not be safe, but for local Haskell programs, it might be useful to run the unbounded interpreter and handle exceptions, timeouts, etc. in an application-specific manner, which the time-bounded interpreter doesn't really make possible. The time-bounded handler needs an additional `MonadIO` constraint because the variant of the interpreter that it uses handles timeouts and other imprecise exceptions that may be thrown by the interpreter. This is unlike any other actions in `Primer.App`, but it's unavoidable due to our particular lazy implementation of the interpreter. (See the comments in the interpreter source for details.) Signed-off-by: Drew Hess <src@drewhess.com>
This test change is a separate commit to the parent commit because it requires a change to `EditAppM`, namely adding a `MonadIO` instance to it. Prior to this change, no action tests required `IO`. Signed-off-by: Drew Hess <src@drewhess.com>
Signed-off-by: Drew Hess <src@drewhess.com>
Signed-off-by: Drew Hess <src@drewhess.com>
|
Hmm, I've discovered during testing this PR with the frontend that the edit: tests added, here are the failures: https://buildkite.com/hackworthltd/primer-ci/builds/80#018efd2d-80ca-40af-a0d2-2b25a5e2c179/16-32 |
|
Actually, it's not just the |
|
The bug is now tracked in #1247 |
|
I think I'm going to merge this after wrapping "expected" around the test failures. There's a substantial amount of work here, in addition to a non-trivial frontend PR in hackworthltd/primer-app#1169, and since it may take some time for me to get up to speed on the interpreter implementation in order to find the issue, I don't want these PRs to languish. Though the functionality here is very nice to have (the interpreter is an order of magnitude or more faster than the step evaluator), we can live without it until I can get to the root of the problem. |
Note that all of these tests that use the interpreter are currently expected to fail (or are commented out because they will fail) due to #1247 Signed-off-by: Drew Hess <src@drewhess.com>
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
The Wasm version of this test needs more time. Signed-off-by: Drew Hess <src@drewhess.com>
Note that we only expose the time-bounded interpreter via the HTTP API, because the non-bounded variant would enable trivial DoS attacks against a Primer service. We do, however, expose the non-bounded variant via the local API, as it might be useful to handle the various exceptions it can throw (and runaway evaluations) in an application-specific way, differently to how the time-bounded variant does it.
Closes #1239.
Note that there's currently a bug in the interpreter that prevents it from reducing top-level definitions that contain applications. See #1247.