Skip to content

Commit

Permalink
Fix config validation for feast.jobs.metrics.host (#662)
Browse files Browse the repository at this point in the history
* Update config validation for feast.jobs.metrics.host

`host` will be used as the parameter to connect to a metrics server e.g StatsD Server. It's not a URL but rather an IP address or hostname. For example StatsD UDP server expects a DNS hostname or IP address and not a URL such as http://10.23.40.1. The UDP server does not understand http application layer protocol.

* Typo in error message
  • Loading branch information
davidheryanto authored Apr 29, 2020
1 parent d3ba7d8 commit 65c2789
Showing 1 changed file with 16 additions and 2 deletions.
18 changes: 16 additions & 2 deletions core/src/main/java/feast/core/config/FeastProperties.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import feast.core.config.FeastProperties.StreamProperties.FeatureStreamOptions;
import feast.core.validators.OneOfStrings;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.*;
import javax.annotation.PostConstruct;
import javax.validation.*;
Expand All @@ -26,7 +28,6 @@
import javax.validation.constraints.Positive;
import lombok.Getter;
import lombok.Setter;
import org.hibernate.validator.constraints.URL;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.info.BuildProperties;
Expand Down Expand Up @@ -91,6 +92,7 @@ public Runner getActiveRunner() {
@Getter
@Setter
public static class Runner {

/** Job runner name. This must be unique. */
String name;

Expand Down Expand Up @@ -173,7 +175,7 @@ public static class MetricsProperties {
private String type;

/* Host of metric sink */
@URL private String host;
private String host;

/* Port of metric sink */
@Positive private int port;
Expand Down Expand Up @@ -221,6 +223,18 @@ public void validate() {
if (!jobMetricViolations.isEmpty()) {
throw new ConstraintViolationException(jobMetricViolations);
}
// Additional custom check for hostname value because there is no built-in Spring annotation
// to validate the value is a DNS resolvable hostname or an IP address.
try {
//noinspection ResultOfMethodCallIgnored
InetAddress.getByName(getJobs().getMetrics().getHost());
} catch (UnknownHostException e) {
throw new IllegalArgumentException(
"Invalid config value for feast.jobs.metrics.host: "
+ getJobs().getMetrics().getHost()
+ ". Make sure it is a valid IP address or DNS hostname e.g. localhost or 10.128.10.40. Error detail: "
+ e.getMessage());
}
}
}
}

0 comments on commit 65c2789

Please sign in to comment.