Initial Support for Remote Evals Devserver#23
Conversation
Andrew Kent (realark)
force-pushed
the
ark/remote-evals
branch
7 times, most recently
from
28b1576 to
6404057
Compare
Andrew Kent (realark)
force-pushed
the
ark/remote-evals
branch
from
6404057 to
2b95212
Compare
Andrew Kent (realark)
requested review from
Matt Perpick (clutchski) and
David Elner (delner)
Andrew Kent (realark)
force-pushed
the
ark/remote-evals
branch
2 times, most recently
from
e234cc2 to
5646de4
Compare
| @BeforeAll | ||
| static void setUp() throws Exception { | ||
| // Set up mock Braintrust API server | ||
| mockApiServer = HttpServer.create(new InetSocketAddress("localhost", MOCK_API_PORT), 0); |
There was a problem hiding this comment.
I have a few a mock http servers in the SDK unit tests. I plan to switch everything over to VCR pretty soon, but this PR is already a beast so I want to do that later
Andrew Kent (realark)
force-pushed
the
ark/remote-evals
branch
from
5646de4 to
257834c
Compare
David Elner (delner)
left a comment
David Elner (delner)
left a comment
There was a problem hiding this comment.
Can you give us a brief description of the feature, the design you went with, and any particular decisions you made? Would be helpful for contextualizing the rest of my review!
| * @param <V> Value type | ||
| */ | ||
| @ThreadSafe | ||
| class LRUCache<K, V> { |
There was a problem hiding this comment.
Interesting. What's this used for? Didn't see any references to it.
Is this generic enough (given the use of templates) that its considered a "Util" as opposed to "devserver" specific? Unfamiliar with Java's patterns for common use components like this.
There was a problem hiding this comment.
It's used in devserver. It caches the auth info (on my phone rn but I think the name is authCache).
This could totally be a generic util. It's package-private right now, so if/when another module needs it I can move it up a package without any breaking changes.
For sure! I'll give you a demo Monday. The context helps |
David Elner (delner)
left a comment
David Elner (delner)
left a comment
There was a problem hiding this comment.
Andrew gave me a demo of this early last week (thanks!) Changes make sense overall!
2 participants
No description provided.