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

chore: Split postgres url in to parts. #157

Merged
merged 6 commits into from
Jul 27, 2022
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
5 changes: 5 additions & 0 deletions helm/templates/query-service-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ data:
type = zookeeper
connectionString = "{{ .Values.queryServiceConfig.data.zookeeperConnectionString }}"
}
{
type = postgres
connectionString = "{{ .Values.queryServiceConfig.data.postgresConnectionString }}"
user = "{{ .Values.queryServiceConfig.data.postgresUser }}"
}
]
{{- if or .Values.handlers .Values.extraHandlers }}
queryRequestHandlersConfig = [
Expand Down
2 changes: 2 additions & 0 deletions helm/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ queryServiceConfig:
name: query-service-config
data:
zookeeperConnectionString: zookeeper:2181/pinot/my-views
postgresConnectionString: jdbc:postgresql://postgres:5432/postgres
postgresUser: postgres
tenantColumnName: tenant_id
attributeClient:
host: attribute-service
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.typesafe.config.Config;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
import lombok.Value;
import lombok.experimental.NonFinal;
Expand Down Expand Up @@ -62,12 +63,26 @@ private RequestHandlerConfig(Config config) {
public static class RequestHandlerClientConfig {
private static final String CONFIG_PATH_TYPE = "type";
private static final String CONFIG_PATH_CONNECTION_STRING = "connectionString";
private static final String CONFIG_PATH_USER = "user";
private static final String CONFIG_PATH_PASSWORD = "password";
String type;
String connectionString;
Optional<String> user;
Optional<String> password;

private RequestHandlerClientConfig(Config config) {
this.type = config.getString(CONFIG_PATH_TYPE);
this.connectionString = config.getString(CONFIG_PATH_CONNECTION_STRING);
if (config.hasPath(CONFIG_PATH_USER)) {
this.user = Optional.of(config.getString(CONFIG_PATH_USER));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

this.user = config.hasPath(CONFIG_PATH_USER) ? Optional.of(config.getString(CONFIG_PATH_USER)) : Optional.empty()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do it in follow up PR

} else {
this.user = Optional.empty();
}
if (config.hasPath(CONFIG_PATH_PASSWORD)) {
this.password = Optional.of(config.getString(CONFIG_PATH_PASSWORD));
} else {
this.password = Optional.empty();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.concurrent.ConcurrentHashMap;
import org.hypertrace.core.query.service.QueryServiceConfig.RequestHandlerClientConfig;

/*
* Factory to create PostgresClient based on postgres jdbc connection.
Expand All @@ -23,12 +24,12 @@ public class PostgresClientFactory {
private PostgresClientFactory() {}

// Create a Postgres Client.
public static PostgresClient createPostgresClient(String postgresCluster, String path)
throws SQLException {
public static PostgresClient createPostgresClient(
String postgresCluster, RequestHandlerClientConfig clientConfig) throws SQLException {
if (!get().containsClient(postgresCluster)) {
synchronized (get()) {
if (!get().containsClient(postgresCluster)) {
get().addPostgresClient(postgresCluster, new PostgresClient(path));
get().addPostgresClient(postgresCluster, new PostgresClient(clientConfig));
}
}
}
Expand Down Expand Up @@ -59,7 +60,12 @@ public static class PostgresClient {

private final Connection connection;

private PostgresClient(String url) throws SQLException {
private PostgresClient(RequestHandlerClientConfig clientConfig) throws SQLException {
String user = clientConfig.getUser().orElseThrow(IllegalArgumentException::new);
String password = clientConfig.getUser().orElseThrow(IllegalArgumentException::new);
String url =
String.format(
"%s?user=%s&password=%s", clientConfig.getConnectionString(), user, password);
LOG.info(
"Trying to create a Postgres client connected to postgres server using url: {}", url);
this.connection = DriverManager.getConnection(url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ public RequestHandler build(RequestHandlerConfig config) {
"Client config requested but not registered: " + config.getClientConfig()));

try {
PostgresClientFactory.createPostgresClient(
config.getName(), clientConfig.getConnectionString());
PostgresClientFactory.createPostgresClient(config.getName(), clientConfig);
} catch (SQLException ex) {
throw new RuntimeException(ex);
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,8 @@ public static void setup() throws Exception {

withEnvironmentVariable(
"POSTGRES_CONNECT_STR",
"jdbc:postgresql://localhost:"
+ postgresqlService.getMappedPort(5432).toString()
+ "/postgres?user=postgres&password=postgres")
String.format(
"jdbc:postgresql://localhost:%s/postgres", postgresqlService.getMappedPort(5432)))
.and("ATTRIBUTE_SERVICE_HOST_CONFIG", attributeService.getHost())
.and("ATTRIBUTE_SERVICE_PORT_CONFIG", attributeService.getMappedPort(9012).toString())
.execute(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ service.config = {
clients = [
{
type = postgres
connectionString = "jdbc:postgresql://localhost:5432/traceable?user=postgres&password=postgres"
connectionString = ${?POSTGRES_CONNECT_STR}
user = postgres
password = postgres
}
]
queryRequestHandlersConfig = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ service.config = {
connectionString = "localhost:2181/pinot/org-views"
connectionString = ${?ZK_CONNECT_STR}
}
{
type = postgres
connectionString = "jdbc:postgresql://localhost:5432/postgres"
connectionString = ${?POSTGRES_CONNECT_STR}
user = postgres
user = ${?POSTGRES_USER}
password = postgres
password = ${?POSTGRES_PASSWORD}
}
]
queryRequestHandlersConfig = [
# Update runtime configuration in helm/values.yaml. Only local test/debug needs the following
Expand Down