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

Commit

Permalink
Complete python gRPC requests when closing (LangStream#690)
Browse files Browse the repository at this point in the history
  • Loading branch information
cbornet authored Nov 6, 2023
1 parent 90b9f32 commit 586bc4c
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void start() throws Exception {
}

@Override
public void setContext(AgentContext context) throws Exception {
public void setContext(AgentContext context) {
this.agentContext = context;
}

Expand Down Expand Up @@ -128,6 +128,7 @@ public void stopChannel(boolean wait) throws Exception {

@Override
public synchronized void close() throws Exception {
stopBeforeRestart();
stopChannel(true);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,21 +100,13 @@ public void process(List<ai.langstream.api.runner.code.Record> records, RecordSi
records.size());
records.forEach(recordSink::emitEmptyList);
} else {
records.forEach(
e -> {
recordSink.emitError(e, stopped);
});
records.forEach(e -> recordSink.emitError(e, stopped));
}
}
}
}
}

@Override
public synchronized void close() throws Exception {
stopBeforeRestart();
}

private SourceRecordAndResult fromGrpc(
ai.langstream.api.runner.code.Record sourceRecord, ProcessorResult result)
throws IOException {
Expand Down Expand Up @@ -202,8 +194,8 @@ protected void stopBeforeRestart() throws Exception {
if (request != null) {
try {
request.onCompleted();
} catch (IllegalStateException ignored) {
log.info("Ignoring error while stopping {}", ignored + "");
} catch (IllegalStateException e) {
log.info("Ignoring error while stopping {}", e + "");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ protected void stopBeforeRestart() throws Exception {
if (request != null) {
try {
request.onCompleted();
} catch (IllegalStateException ignored) {
log.info("Ignoring error while stopping {}", ignored + "");
} catch (IllegalStateException e) {
log.info("Ignoring error while stopping {}", e + "");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public List<Record> read() throws Exception {
}

@Override
public void permanentFailure(Record record, Exception error) throws Exception {
public void permanentFailure(Record record, Exception error) {
if (record instanceof GrpcAgentRecord grpcAgentRecord) {
request.onNext(
SourceRequest.newBuilder()
Expand Down Expand Up @@ -156,8 +156,8 @@ protected void stopBeforeRestart() throws Exception {
if (request != null) {
try {
request.onCompleted();
} catch (IllegalStateException ignored) {
log.info("Ignoring error while stopping {}", ignored + "");
} catch (IllegalStateException e) {
log.info("Ignoring error while stopping {}", e + "");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,17 @@ public void start() throws Exception {

@Override
public synchronized void close() throws Exception {
if (server != null) server.close(false);
super.close();
if (server != null) {
server.close(false);
}
}

@Override
protected synchronized void stopBeforeRestart() throws Exception {
super.stopBeforeRestart();
if (server != null) server.close(true);
if (server != null) {
server.close(true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,17 @@ public void start() throws Exception {

@Override
public synchronized void close() throws Exception {
if (server != null) server.close(false);
super.close();
if (server != null) {
server.close(false);
}
}

@Override
protected synchronized void stopBeforeRestart() throws Exception {
super.stopBeforeRestart();
if (server != null) server.close(true);
if (server != null) {
server.close(true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,17 @@ public void start() throws Exception {

@Override
public synchronized void close() throws Exception {
if (server != null) server.close(false);
super.close();
if (server != null) {
server.close(false);
}
}

@Override
protected synchronized void stopBeforeRestart() throws Exception {
super.stopBeforeRestart();
if (server != null) server.close(true);
if (server != null) {
server.close(true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@
public class GrpcAgentProcessorTest {
private Server server;
private ManagedChannel channel;
private GrpcAgentProcessor processor;
private TestAgentContext context;
private final AtomicInteger schemaCounter = new AtomicInteger(0);

private final AgentServiceGrpc.AgentServiceImplBase testProcessorService =
Expand Down Expand Up @@ -128,7 +130,9 @@ public void onNext(ProcessorRequest request) {
public void onError(Throwable throwable) {}

@Override
public void onCompleted() {}
public void onCompleted() {
response.onCompleted();
}
};
}
};
Expand All @@ -144,6 +148,10 @@ public void setUp() throws Exception {
.start();

channel = InProcessChannelBuilder.forName(serverName).directExecutor().build();
processor = new GrpcAgentProcessor(channel);
context = new TestAgentContext();
processor.setContext(context);
processor.start();
schemaCounter.set(0);
}

Expand Down Expand Up @@ -175,9 +183,6 @@ private static Stream<Arguments> primitives() {
@ParameterizedTest
@MethodSource("primitives")
void testProcess(Object value, Object key, Object header) throws Exception {
GrpcAgentProcessor processor = new GrpcAgentProcessor(channel);
processor.setContext(new TestAgentContext());
processor.start();
Record inputRecord =
SimpleRecord.builder()
.value(value)
Expand All @@ -188,26 +193,17 @@ void testProcess(Object value, Object key, Object header) throws Exception {
.build();
assertProcessSuccessful(processor, inputRecord);
assertProcessSuccessful(processor, inputRecord);
processor.close();
}

@Test
void testEmpty() throws Exception {
GrpcAgentProcessor processor = new GrpcAgentProcessor(channel);
TestAgentContext context = new TestAgentContext();
processor.setContext(context);
processor.start();
assertProcessSuccessful(processor, SimpleRecord.builder().build());
assertFalse(context.failureCalled.await(1, TimeUnit.SECONDS));
processor.close();
}

@Test
void testFailingRecord() throws Exception {
GrpcAgentProcessor processor = new GrpcAgentProcessor(channel);
Record inputRecord = SimpleRecord.builder().origin("failing-origin").build();
processor.setContext(new TestAgentContext());
processor.start();
CompletableFuture<Void> op = new CompletableFuture<>();
processor.process(
List.of(inputRecord),
Expand All @@ -223,23 +219,17 @@ void testFailingRecord() throws Exception {
op.complete(null);
});
op.get(5, TimeUnit.SECONDS);
processor.close();
}

@ParameterizedTest
@ValueSource(
strings = {"failing-server", "completing-server", "wrong-record-id", "wrong-schema-id"})
void testServerError(String origin) throws Exception {
GrpcAgentProcessor processor = new GrpcAgentProcessor(channel);
Record inputRecord = SimpleRecord.builder().origin(origin).build();

TestAgentContext testAgentContext = new TestAgentContext();
processor.setContext(testAgentContext);
processor.start();
processor.process(List.of(inputRecord), result -> {});

assertTrue(testAgentContext.failureCalled.await(1, TimeUnit.SECONDS));
processor.close();
assertTrue(context.failureCalled.await(1, TimeUnit.SECONDS));
}

@Test
Expand All @@ -254,9 +244,6 @@ void testAvroAndSchema() throws Exception {
.endRecord();
GenericData.Record avroRecord = new GenericData.Record(schema);
avroRecord.put("testField", "test-string");
GrpcAgentProcessor processor = new GrpcAgentProcessor(channel);
processor.setContext(new TestAgentContext());
processor.start();
Record inputRecord =
SimpleRecord.builder()
.value(avroRecord)
Expand All @@ -267,14 +254,10 @@ void testAvroAndSchema() throws Exception {
assertProcessSuccessful(processor, inputRecord);
assertProcessSuccessful(processor, inputRecord);
assertEquals(1, schemaCounter.get());
processor.close();
}

@Test
void testInfo() throws Exception {
GrpcAgentProcessor processor = new GrpcAgentProcessor(channel);
processor.setContext(new TestAgentContext());
processor.start();
Map<String, Object> info = processor.buildAdditionalInfo();
assertEquals("test-info-value", info.get("test-info-key"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,9 @@ public void onNext(SinkRequest request) {
public void onError(Throwable throwable) {}

@Override
public void onCompleted() {}
public void onCompleted() {
responseObserver.onCompleted();
}
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@
public class GrpcAgentSourceTest {
private Server server;
private ManagedChannel channel;

private GrpcAgentSource source;
private TestAgentContext context;
private final TestSourceService testSourceService = new TestSourceService();

@BeforeEach
Expand All @@ -67,6 +68,10 @@ public void setUp() throws Exception {
.start();

channel = InProcessChannelBuilder.forName(serverName).directExecutor().build();
source = new GrpcAgentSource(channel);
context = new TestAgentContext();
source.setContext(context);
source.start();
}

@AfterEach
Expand All @@ -79,10 +84,6 @@ public void tearDown() throws Exception {

@Test
void testCommit() throws Exception {
GrpcAgentSource source = new GrpcAgentSource(channel);
TestAgentContext context = new TestAgentContext();
source.setContext(context);
source.start();
List<Record> read = readRecords(source, 3);
source.commit(List.of(read.get(0)));
assertFalse(context.failureCalled.await(1, TimeUnit.SECONDS));
Expand All @@ -93,10 +94,6 @@ void testCommit() throws Exception {

@Test
void testSourceGrpcError() throws Exception {
GrpcAgentSource source = new GrpcAgentSource(channel);
TestAgentContext context = new TestAgentContext();
source.setContext(context);
source.start();
List<Record> read = readRecords(source, 3);
source.commit(List.of(read.get(1)));
assertTrue(context.failureCalled.await(1, TimeUnit.SECONDS));
Expand All @@ -105,10 +102,6 @@ void testSourceGrpcError() throws Exception {

@Test
void testSourceGrpcCompletedUnexpectedly() throws Exception {
GrpcAgentSource source = new GrpcAgentSource(channel);
TestAgentContext context = new TestAgentContext();
source.setContext(context);
source.start();
List<Record> read = readRecords(source, 3);
source.commit(List.of(read.get(2)));
assertTrue(context.failureCalled.await(1, TimeUnit.SECONDS));
Expand All @@ -117,9 +110,6 @@ void testSourceGrpcCompletedUnexpectedly() throws Exception {

@Test
void testAvroAndSchema() throws Exception {
GrpcAgentSource source = new GrpcAgentSource(channel);
source.setContext(new TestAgentContext());
source.start();
List<Record> read = readRecords(source, 1);
GenericRecord record = (GenericRecord) read.get(0).value();
assertEquals("test-string", record.get("testField").toString());
Expand All @@ -128,9 +118,6 @@ void testAvroAndSchema() throws Exception {

@Test
void testPermanentFailure() throws Exception {
GrpcAgentSource source = new GrpcAgentSource(channel);
source.setContext(new TestAgentContext());
source.start();
List<Record> read = readRecords(source, 1);
source.permanentFailure(read.get(0), new RuntimeException("permanent-failure"));
assertEquals(testSourceService.permanentFailure.getRecordId(), 42);
Expand Down Expand Up @@ -227,7 +214,9 @@ public void onNext(SourceRequest request) {
public void onError(Throwable throwable) {}

@Override
public void onCompleted() {}
public void onCompleted() {
responseObserver.onCompleted();
}
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ default void close() throws Exception {}
/**
* Gracefully restart the agent.
*
* @throws Exception
* @throws Exception if an error occurs
*/
default void restart() throws Exception {}
}

0 comments on commit 586bc4c

Please sign in to comment.