Skip to content

Commit 3231952

Browse files
authored
Merge pull request #36969 from fabrik-bot/agent/oss-vespa-engine-vespa-10842-897c31fe
Let Vespa HTTP client exit with non-zero return code on parsing errors
2 parents ce060d0 + 6ce7848 commit 3231952

4 files changed

Lines changed: 179 additions & 0 deletions

File tree

vespa-feed-client-cli/src/main/java/ai/vespa/feed/client/impl/CliArguments.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ class CliArguments {
6565
private static final String COMPRESSION = "compression";
6666
private static final String LOG_CONFIG_OPTION = "log-config";
6767
private static final String INITIAL_INFLIGHT_FACTOR_OPTION = "initial-inflight-factor";
68+
private static final String EXIT_ON_FEED_ERRORS_OPTION = "exit-on-feed-errors";
6869

6970
private final CommandLine arguments;
7071

@@ -232,6 +233,8 @@ private Optional<Path> fileValue(String option) throws CliArgumentsException {
232233

233234
OptionalInt initialInflightFactor() throws CliArgumentsException { return intValue(INITIAL_INFLIGHT_FACTOR_OPTION); }
234235

236+
boolean exitOnFeedErrorsEnabled() { return has(EXIT_ON_FEED_ERRORS_OPTION); }
237+
235238
private Optional<String> stringValue(String option) { return Optional.ofNullable(arguments.getOptionValue(option)); }
236239

237240
private OptionalDouble doubleValue(String option) throws CliArgumentsException {
@@ -396,6 +399,10 @@ private static Options createOptions() {
396399
.desc("Multiplier for minInflight to determine the initial targetInflight")
397400
.hasArg()
398401
.type(Number.class)
402+
.build())
403+
.addOption(Option.builder()
404+
.longOpt(EXIT_ON_FEED_ERRORS_OPTION)
405+
.desc("Exit with non-zero exit code if any feed operation fails")
399406
.build());
400407
}
401408

vespa-feed-client-cli/src/main/java/ai/vespa/feed/client/impl/CliClient.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ public void onError(FeedException error) {
130130

131131
printBenchmarkResult(System.nanoTime() - startNanos, successes.get(), failures.get(), feedClient.stats(), cliArgs.benchmarkModeEnabled() ? systemOut : systemError);
132132
if (fatal.get() != null) throw fatal.get();
133+
if (cliArgs.exitOnFeedErrorsEnabled() && failures.get() > 0) return 1;
133134
}
134135
return 0;
135136
}

vespa-feed-client-cli/src/test/java/ai/vespa/feed/client/impl/CliClientTest.java

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,38 @@
11
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
22
package ai.vespa.feed.client.impl;
33

4+
import ai.vespa.feed.client.DocumentId;
5+
import ai.vespa.feed.client.FeedClient;
6+
import ai.vespa.feed.client.FeedClientBuilder;
7+
import ai.vespa.feed.client.OperationParameters;
8+
import ai.vespa.feed.client.OperationStats;
9+
import ai.vespa.feed.client.Result;
10+
import ai.vespa.feed.client.ResultException;
11+
import org.junit.jupiter.api.AfterEach;
12+
import org.junit.jupiter.api.BeforeEach;
413
import org.junit.jupiter.api.Test;
514

15+
import java.io.ByteArrayInputStream;
616
import java.io.IOException;
717
import java.io.InputStream;
18+
import java.io.OutputStream;
19+
import java.io.PrintStream;
20+
import java.lang.reflect.Constructor;
21+
import java.lang.reflect.Method;
22+
import java.net.URI;
823
import java.nio.charset.StandardCharsets;
24+
import java.nio.file.Path;
25+
import java.security.PrivateKey;
26+
import java.security.cert.X509Certificate;
27+
import java.time.Duration;
28+
import java.util.Collection;
29+
import java.util.List;
30+
import java.util.Map;
31+
import java.util.Optional;
932
import java.util.Random;
33+
import java.util.concurrent.CompletableFuture;
1034
import java.util.concurrent.atomic.AtomicInteger;
35+
import java.util.function.Supplier;
1136

1237
import static org.junit.jupiter.api.Assertions.assertEquals;
1338

@@ -16,6 +41,23 @@
1641
*/
1742
class CliClientTest {
1843

44+
private Supplier<FeedClientBuilder> originalSupplier;
45+
46+
@SuppressWarnings("unchecked")
47+
@BeforeEach
48+
void saveOriginalFeedClientBuilderSupplier() throws Exception {
49+
var helperClass = Class.forName("ai.vespa.feed.client.Helper");
50+
var field = helperClass.getDeclaredField("feedClientBuilderSupplier");
51+
field.setAccessible(true);
52+
var atomicRef = (java.util.concurrent.atomic.AtomicReference<Supplier<FeedClientBuilder>>) field.get(null);
53+
originalSupplier = atomicRef.get();
54+
}
55+
56+
@AfterEach
57+
void restoreFeedClientBuilderSupplier() {
58+
FeedClientBuilder.setFeedClientBuilderSupplier(originalSupplier);
59+
}
60+
1961
@Test
2062
void testDummyStream() throws IOException {
2163
AtomicInteger count = new AtomicInteger(3);
@@ -29,4 +71,131 @@ void testDummyStream() throws IOException {
2971
new String(buffer, 0, offset, StandardCharsets.UTF_8));
3072
}
3173

74+
@Test
75+
void exit_code_is_zero_without_flag_even_when_there_are_feed_failures() throws Exception {
76+
FeedClientBuilder.setFeedClientBuilderSupplier(() -> new StubFeedClientBuilder(false));
77+
int exitCode = runCli("--endpoint", "https://localhost:8080", "--stdin");
78+
assertEquals(0, exitCode);
79+
}
80+
81+
@Test
82+
void exit_code_is_one_with_flag_when_there_are_feed_failures() throws Exception {
83+
FeedClientBuilder.setFeedClientBuilderSupplier(() -> new StubFeedClientBuilder(false));
84+
int exitCode = runCli("--endpoint", "https://localhost:8080", "--stdin", "--exit-on-feed-errors");
85+
assertEquals(1, exitCode);
86+
}
87+
88+
@Test
89+
void exit_code_is_zero_with_flag_when_there_are_no_feed_failures() throws Exception {
90+
FeedClientBuilder.setFeedClientBuilderSupplier(() -> new StubFeedClientBuilder(true));
91+
int exitCode = runCli("--endpoint", "https://localhost:8080", "--stdin", "--exit-on-feed-errors");
92+
assertEquals(0, exitCode);
93+
}
94+
95+
private static int runCli(String... args) throws Exception {
96+
String feed = "[ { \"put\": \"id:test:test::doc1\", \"fields\": { \"test\": \"value\" } } ]";
97+
InputStream stdin = new ByteArrayInputStream(feed.getBytes(StandardCharsets.UTF_8));
98+
PrintStream devNull = new PrintStream(OutputStream.nullOutputStream());
99+
CliClient client = newCliClient(devNull, devNull, stdin);
100+
Method run = CliClient.class.getDeclaredMethod("run", String[].class);
101+
run.setAccessible(true);
102+
return (int) run.invoke(client, (Object) args);
103+
}
104+
105+
private static CliClient newCliClient(PrintStream out, PrintStream err, InputStream in) throws Exception {
106+
Constructor<CliClient> ctor = CliClient.class.getDeclaredConstructor(PrintStream.class, PrintStream.class, InputStream.class);
107+
ctor.setAccessible(true);
108+
return ctor.newInstance(out, err, in);
109+
}
110+
111+
static class StubFeedClientBuilder implements FeedClientBuilder {
112+
113+
private final boolean succeed;
114+
115+
StubFeedClientBuilder(boolean succeed) {
116+
this.succeed = succeed;
117+
}
118+
119+
@Override public FeedClient build() { return new StubFeedClient(succeed); }
120+
@Override public FeedClientBuilder setEndpointUris(List<URI> endpoints) { return this; }
121+
@Override public FeedClientBuilder setConnectionsPerEndpoint(int max) { return this; }
122+
@Override public FeedClientBuilder setMaxStreamPerConnection(int max) { return this; }
123+
@Override public FeedClientBuilder setConnectionTimeToLive(Duration ttl) { return this; }
124+
@Override public FeedClientBuilder setSslContext(javax.net.ssl.SSLContext context) { return this; }
125+
@Override public FeedClientBuilder setHostnameVerifier(javax.net.ssl.HostnameVerifier verifier) { return this; }
126+
@Override public FeedClientBuilder setProxyHostnameVerifier(javax.net.ssl.HostnameVerifier verifier) { return this; }
127+
@Override public FeedClientBuilder noBenchmarking() { return this; }
128+
@Override public FeedClientBuilder addRequestHeader(String name, String value) { return this; }
129+
@Override public FeedClientBuilder addRequestHeader(String name, Supplier<String> valueSupplier) { return this; }
130+
@Override public FeedClientBuilder addProxyRequestHeader(String name, String value) { return this; }
131+
@Override public FeedClientBuilder addProxyRequestHeader(String name, Supplier<String> valueSupplier) { return this; }
132+
@Override public FeedClientBuilder setRetryStrategy(FeedClient.RetryStrategy strategy) { return this; }
133+
@Override public FeedClientBuilder setCircuitBreaker(FeedClient.CircuitBreaker breaker) { return this; }
134+
@Override public FeedClientBuilder setCertificate(Path certificatePemFile, Path privateKeyPemFile) { return this; }
135+
@Override public FeedClientBuilder setCertificate(Collection<X509Certificate> certificate, PrivateKey privateKey) { return this; }
136+
@Override public FeedClientBuilder setCertificate(X509Certificate certificate, PrivateKey privateKey) { return this; }
137+
@Override public FeedClientBuilder setDryrun(boolean enabled) { return this; }
138+
@Override public FeedClientBuilder setSpeedTest(boolean enabled) { return this; }
139+
@Override public FeedClientBuilder setCaCertificatesFile(Path caCertificatesFile) { return this; }
140+
@Override public FeedClientBuilder setProxyCaCertificatesFile(Path caCertificatesFile) { return this; }
141+
@Override public FeedClientBuilder setCaCertificates(Collection<X509Certificate> caCertificates) { return this; }
142+
@Override public FeedClientBuilder setProxyCaCertificates(Collection<X509Certificate> caCertificates) { return this; }
143+
@Override public FeedClientBuilder setProxy(URI uri) { return this; }
144+
@Override public FeedClientBuilder setCompression(FeedClientBuilder.Compression compression) { return this; }
145+
@Override public FeedClientBuilder setInitialInflightFactor(int factor) { return this; }
146+
}
147+
148+
static class StubFeedClient implements FeedClient {
149+
150+
private final boolean succeed;
151+
152+
StubFeedClient(boolean succeed) {
153+
this.succeed = succeed;
154+
}
155+
156+
@Override
157+
public CompletableFuture<Result> put(DocumentId documentId, String documentJson, OperationParameters params) {
158+
CompletableFuture<Result> f = new CompletableFuture<>();
159+
if (succeed) {
160+
f.complete(new StubResult(documentId));
161+
} else {
162+
f.completeExceptionally(new ResultException(documentId, "mocked feed failure", null));
163+
}
164+
return f;
165+
}
166+
167+
@Override
168+
public CompletableFuture<Result> update(DocumentId documentId, String updateJson, OperationParameters params) {
169+
return put(documentId, updateJson, params);
170+
}
171+
172+
@Override
173+
public CompletableFuture<Result> remove(DocumentId documentId, OperationParameters params) {
174+
return put(documentId, null, params);
175+
}
176+
177+
@Override
178+
public OperationStats stats() {
179+
return new OperationStats(0, 0, 0, 0, 0, 0, 0, 0, 0, Map.of());
180+
}
181+
182+
@Override public void resetStats() { }
183+
@Override public FeedClient.CircuitBreaker.State circuitBreakerState() { return FeedClient.CircuitBreaker.State.CLOSED; }
184+
@Override public void close(boolean graceful) { }
185+
}
186+
187+
static class StubResult implements Result {
188+
189+
private final DocumentId documentId;
190+
191+
StubResult(DocumentId documentId) {
192+
this.documentId = documentId;
193+
}
194+
195+
@Override public Result.Type type() { return Result.Type.success; }
196+
@Override public DocumentId documentId() { return documentId; }
197+
@Override public Optional<String> resultMessage() { return Optional.empty(); }
198+
@Override public Optional<String> traceMessage() { return Optional.empty(); }
199+
}
200+
32201
}

vespa-feed-client-cli/src/test/resources/help.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ Vespa feed client
1919
1ms, instead of sending it
2020
across the network
2121
--endpoint <arg> URI to feed endpoint
22+
--exit-on-feed-errors Exit with non-zero exit code if
23+
any feed operation fails
2224
--file <arg> Path to feed file in JSON format
2325
--header <arg> HTTP header on the form 'Name:
2426
value'

0 commit comments

Comments
 (0)