Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): provide descriptive server start error #4171

Merged
merged 2 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 39 additions & 13 deletions cli/src/main/java/io/kestra/cli/App.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import io.kestra.cli.commands.flows.FlowCommand;
import io.kestra.cli.commands.namespaces.NamespaceCommand;
import io.kestra.cli.commands.plugins.PluginCommand;
import io.kestra.cli.commands.servers.AbstractServerCommand;
import io.kestra.cli.commands.servers.ServerCommand;
import io.kestra.cli.commands.sys.SysCommand;
import io.kestra.cli.commands.templates.TemplateCommand;
Expand All @@ -13,16 +14,14 @@
import io.micronaut.context.ApplicationContextBuilder;
import io.micronaut.context.env.Environment;
import io.micronaut.core.annotation.Introspected;
import lombok.extern.slf4j.Slf4j;
import org.slf4j.bridge.SLF4JBridgeHandler;
import picocli.CommandLine;

import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.*;
import java.util.concurrent.Callable;

@CommandLine.Command(
Expand All @@ -44,6 +43,7 @@
NamespaceCommand.class,
}
)
@Slf4j
@Introspected
public class App implements Callable<Integer> {
public static void main(String[] args) {
Expand Down Expand Up @@ -79,9 +79,7 @@ protected static void execute(Class<?> cls, String... args) {
* @param args args passed to java app
* @return the application context created
*/
protected static ApplicationContext applicationContext(Class<?> mainClass,
String[] args) {

protected static ApplicationContext applicationContext(Class<?> mainClass, String[] args) {
ApplicationContextBuilder builder = ApplicationContext
.builder()
.mainClass(mainClass)
Expand All @@ -96,15 +94,15 @@ protected static ApplicationContext applicationContext(Class<?> mainClass,
Class<?> cls = commandLine.getCommandSpec().userObject().getClass();

if (AbstractCommand.class.isAssignableFrom(cls)) {
// if class have propertiesFromConfig, add configuration files
builder.properties(getPropertiesFromMethod(cls, "propertiesFromConfig", commandLine.getCommandSpec().userObject()));

Map<String, Object> properties = new HashMap<>();

// if class have propertiesFromConfig, add configuration files
addPropertiesFromMethod(properties, cls, "propertiesFromConfig", commandLine.getCommandSpec().userObject());
// if class have propertiesOverrides, add force properties for this class
Map<String, Object> propertiesOverrides = getPropertiesFromMethod(cls, "propertiesOverrides", null);
if (propertiesOverrides != null) {
properties.putAll(propertiesOverrides);
addPropertiesFromMethod(properties, cls, "propertiesOverrides", null);

if (AbstractServerCommand.class.isAssignableFrom(cls)) {
validateServerCmdConfig(properties);
}

// custom server configuration
Expand All @@ -123,6 +121,34 @@ protected static ApplicationContext applicationContext(Class<?> mainClass,
return builder.build();
}

private static void addPropertiesFromMethod(Map<String, Object> properties, Class<?> cls, String methodName, Object instance) {
Map<String, Object> propertiesFromMethod = getPropertiesFromMethod(cls, methodName, instance);
if (propertiesFromMethod != null) {
properties.putAll(propertiesFromMethod);
}
}

private static void validateServerCmdConfig(Map<String, Object> propertiesFromConfig) {
final Map<String, String> requiredProperties = Map.of(
"kestra.queue.type", "https://kestra.io/docs/configuration-guide/setup#queue-configuration",
"kestra.repository.type", "https://kestra.io/docs/configuration-guide/setup#repository-configuration",
"kestra.storage.type", "https://kestra.io/docs/configuration-guide/setup#internal-storage-configuration"
);

final List<Map.Entry<String, String>> missingProperties = requiredProperties.entrySet().stream()
.filter((property) -> !propertiesFromConfig.containsKey(property.getKey()))
.toList();

missingProperties.forEach(property -> log.error("""
Server configuration requires the '{}' property to be defined.
For more details, please follow the official setup guide at: {}""", property.getKey(), property.getValue())
);

if (!missingProperties.isEmpty()) {
throw new AbstractServerCommand.ServerCommandException("Incomplete server configuration - missing required properties");
}
}

@SuppressWarnings("unchecked")
private static <T> T getPropertiesFromMethod(Class<?> cls, String methodName, Object instance) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,18 @@
import io.kestra.cli.AbstractCommand;
import picocli.CommandLine;

import java.io.Serial;

abstract public class AbstractServerCommand extends AbstractCommand implements ServerCommandInterface {
@CommandLine.Option(names = {"--port"}, description = "the port to bind")
Integer serverPort;

public static class ServerCommandException extends RuntimeException {
@Serial
private static final long serialVersionUID = 1L;

public ServerCommandException(String errorMessage) {
super(errorMessage);
}
}
}
20 changes: 20 additions & 0 deletions cli/src/test/java/AppTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import io.kestra.cli.commands.servers.AbstractServerCommand;
import io.micronaut.configuration.picocli.PicocliRunner;
import io.micronaut.context.ApplicationContext;
import io.micronaut.context.env.Environment;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import io.kestra.cli.App;

Expand All @@ -9,8 +12,20 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class AppTest {
@BeforeAll
public static void setUp() {
// Don't make any assumptions about $HOME
System.setProperty("user.home", "/foo");
}

@AfterAll
public static void tearDown() {
System.clearProperty("user.home");
}

@Test
public void testHelp() {
ByteArrayOutputStream out = new ByteArrayOutputStream();
Expand All @@ -22,4 +37,9 @@ public void testHelp() {
assertThat(out.toString(), containsString("kestra"));
}
}

@Test
public void testSeverCommandValidation() {
assertThrows(AbstractServerCommand.ServerCommandException.class, () -> App.main(new String[]{"server", "webserver"}));
}
}
Loading