-
Notifications
You must be signed in to change notification settings - Fork 174
Add support for environment config #2660
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
base: master
Are you sure you want to change the base?
Changes from all commits
1187ce5
8a83e3a
89bd5fe
caf6191
e71764f
682433b
60f9605
a36af6c
26c6a79
274493e
9d28fe9
96e853b
b0eb6da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
description = '''Temporal Java SDK Environment Config Module''' | ||
|
||
dependencies { | ||
// this module shouldn't carry temporal-sdk with it, especially for situations when users may be using a shaded artifact | ||
compileOnly project(':temporal-sdk') | ||
|
||
implementation "com.fasterxml.jackson.dataformat:jackson-dataformat-toml:${jacksonVersion}" | ||
testImplementation project(":temporal-testing") | ||
testImplementation "junit:junit:${junitVersion}" | ||
testImplementation "org.mockito:mockito-core:${mockitoVersion}" | ||
|
||
testRuntimeOnly group: 'ch.qos.logback', name: 'logback-classic', version: "${logbackVersion}" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,160 @@ | ||
package io.temporal.envconfig; | ||
|
||
import com.fasterxml.jackson.databind.ObjectReader; | ||
import com.fasterxml.jackson.databind.ObjectWriter; | ||
import com.fasterxml.jackson.dataformat.toml.TomlMapper; | ||
import io.temporal.common.Experimental; | ||
import java.io.*; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
|
||
/** ClientConfig represents a client config file. */ | ||
@Experimental | ||
public class ClientConfig { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hrmm, ideally we could have one static call on client config in the SDKs that gives you what you need to connect a client. With Java this is admittedly harder because 1) stubs are separate from client, and 2) Java doesn't support a native tuple to return two things at once. I wonder if there is any creative approach we can think of here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only thing I could think of was have a method that gives you a full client that takes a few options customizers to set additional options on the service stub options and client options. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think what you have here is good enough There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know it sounds pedantic, but can we also expose static calls for getting the default profile stuff to connect. Specifically, can we have: public static WorkflowServiceStubsOptions loadWorkflowServiceStubsOptions() and public static WorkflowClientOptions loadWorkflowClientOptions() on this class? Or maybe named There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So we wan't a short cut to call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we have been adding this to other SDKs. But it's not a big deal if we don't do it. |
||
|
||
/** Get the default config file path: $HOME/.config/temporal/temporal.toml */ | ||
private static String getDefaultConfigFilePath() { | ||
String userDir = System.getProperty("user.home"); | ||
if (userDir == null || userDir.isEmpty()) { | ||
throw new RuntimeException("failed getting user home directory"); | ||
} | ||
return userDir + "/.config/temporal/temporal.toml"; | ||
} | ||
|
||
/** | ||
* Load all client profiles from given sources. | ||
* | ||
* <p>This does not apply environment variable overrides to the profiles, it only uses an | ||
* environment variable to find the default config file path (TEMPORAL_CONFIG_FILE). To get a | ||
* single profile with environment variables applied, use {@link ClientConfigProfile#load}. | ||
*/ | ||
public static ClientConfig load() throws IOException { | ||
return load(LoadClientConfigOptions.newBuilder().build()); | ||
} | ||
|
||
/** | ||
* Load all client profiles from given sources. | ||
* | ||
* <p>This does not apply environment variable overrides to the profiles, it only uses an | ||
* environment variable to find the default config file path (TEMPORAL_CONFIG_FILE). To get a | ||
* single profile with environment variables applied, use {@link ClientConfigProfile#load}. | ||
* | ||
* @param options options to control loading the config | ||
* @throws IOException if the config file cannot be read or parsed | ||
*/ | ||
public static ClientConfig load(LoadClientConfigOptions options) throws IOException { | ||
ObjectReader reader = new TomlMapper().readerFor(ClientConfigToml.TomlClientConfig.class); | ||
if (options.isStrictConfigFile()) { | ||
reader = | ||
reader.withFeatures( | ||
com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); | ||
} else { | ||
reader = | ||
reader.withoutFeatures( | ||
com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); | ||
} | ||
if (options.getConfigFileData() != null && options.getConfigFileData().length > 0) { | ||
if (options.getConfigFilePath() != null && !options.getConfigFilePath().isEmpty()) { | ||
throw new IllegalArgumentException( | ||
"Cannot have both ConfigFileData and ConfigFilePath set"); | ||
} | ||
ClientConfigToml.TomlClientConfig result = reader.readValue(options.getConfigFileData()); | ||
return new ClientConfig(ClientConfigToml.getClientProfiles(result)); | ||
} else { | ||
// Get file name which is either set value, env var, or default path | ||
String file = options.getConfigFilePath(); | ||
if (file == null || file.isEmpty()) { | ||
Map<String, String> env = options.getEnvOverrides(); | ||
if (env == null) { | ||
env = System.getenv(); | ||
} | ||
// Unlike env vars for the config values, empty and unset env var | ||
// for config file path are both treated as unset | ||
file = env.get("TEMPORAL_CONFIG_FILE"); | ||
} | ||
if (file == null || file.isEmpty()) { | ||
file = getDefaultConfigFilePath(); | ||
} | ||
ClientConfigToml.TomlClientConfig result = reader.readValue(new File(file)); | ||
return new ClientConfig(ClientConfigToml.getClientProfiles(result)); | ||
} | ||
} | ||
|
||
/** | ||
* Load client config from given TOML data. | ||
* | ||
* @param tomlData TOML data to parse | ||
* @return the parsed client config | ||
* @throws IOException if the TOML data cannot be parsed | ||
*/ | ||
public static ClientConfig fromToml(byte[] tomlData) throws IOException { | ||
return fromToml(tomlData, ClientConfigFromTomlOptions.getDefaultInstance()); | ||
} | ||
|
||
/** | ||
* Load client config from given TOML data. | ||
* | ||
* @param tomlData TOML data to parse | ||
* @param options options to control parsing the TOML data | ||
* @return the parsed client config | ||
* @throws IOException if the TOML data cannot be parsed | ||
*/ | ||
public static ClientConfig fromToml(byte[] tomlData, ClientConfigFromTomlOptions options) | ||
throws IOException { | ||
ObjectReader reader = new TomlMapper().readerFor(ClientConfigToml.TomlClientConfig.class); | ||
if (options.isStrictConfigFile()) { | ||
reader = | ||
reader.withFeatures( | ||
com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); | ||
} else { | ||
reader = | ||
reader.withoutFeatures( | ||
com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); | ||
} | ||
ClientConfigToml.TomlClientConfig result = reader.readValue(tomlData); | ||
return new ClientConfig(ClientConfigToml.getClientProfiles(result)); | ||
} | ||
|
||
/** | ||
* Convert the client config to TOML data. Encoding is UTF-8. | ||
* | ||
* @param config the client config to convert | ||
* @return the TOML data as bytes | ||
* @apiNote The output will not be identical to the input if the config was loaded from a file | ||
* because comments and formatting are not preserved. | ||
*/ | ||
public static byte[] toTomlAsBytes(ClientConfig config) throws IOException { | ||
ObjectWriter writer = new TomlMapper().writerFor(ClientConfigToml.TomlClientConfig.class); | ||
return writer.writeValueAsBytes( | ||
new ClientConfigToml.TomlClientConfig( | ||
ClientConfigToml.fromClientProfiles(config.getProfiles()))); | ||
} | ||
|
||
public ClientConfig(Map<String, ClientConfigProfile> profiles) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May want to use a builder here for consistency |
||
this.profiles = profiles; | ||
} | ||
|
||
private final Map<String, ClientConfigProfile> profiles; | ||
|
||
/** All profiles loaded from the config file, may be empty but never null. */ | ||
public Map<String, ClientConfigProfile> getProfiles() { | ||
return profiles; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (o == null || getClass() != o.getClass()) return false; | ||
ClientConfig that = (ClientConfig) o; | ||
return Objects.equals(profiles, that.profiles); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hashCode(profiles); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "ClientConfig{" + "profiles=" + profiles + '}'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
package io.temporal.envconfig; | ||
|
||
/** | ||
* Options for parsing a client config toml file in {@link ClientConfig#fromToml(byte[], | ||
* ClientConfigFromTomlOptions)} | ||
*/ | ||
public class ClientConfigFromTomlOptions { | ||
/** Create a builder for {@link ClientConfigFromTomlOptions}. */ | ||
public static Builder newBuilder() { | ||
return new ClientConfigFromTomlOptions.Builder(); | ||
} | ||
|
||
/** Create a builder from an existing {@link ClientConfigFromTomlOptions}. */ | ||
public static Builder newBuilder(ClientConfigFromTomlOptions options) { | ||
return new Builder(options); | ||
} | ||
|
||
/** Returns a default instance of {@link ClientConfigFromTomlOptions} with all fields unset. */ | ||
public static ClientConfigFromTomlOptions getDefaultInstance() { | ||
return new ClientConfigFromTomlOptions.Builder().build(); | ||
} | ||
|
||
private final Boolean strictConfigFile; | ||
|
||
private ClientConfigFromTomlOptions(Boolean strictConfigFile) { | ||
this.strictConfigFile = strictConfigFile; | ||
} | ||
|
||
public Boolean isStrictConfigFile() { | ||
return strictConfigFile; | ||
} | ||
|
||
public static class Builder { | ||
private Boolean strictConfigFile = false; | ||
|
||
private Builder() {} | ||
|
||
private Builder(ClientConfigFromTomlOptions options) { | ||
this.strictConfigFile = options.strictConfigFile; | ||
} | ||
|
||
/** | ||
* When true, the parser will fail if the config file contains unknown fields. Default is false. | ||
*/ | ||
public Builder setStrictConfigFile(Boolean strictConfigFile) { | ||
this.strictConfigFile = strictConfigFile; | ||
return this; | ||
} | ||
|
||
public ClientConfigFromTomlOptions build() { | ||
return new ClientConfigFromTomlOptions(strictConfigFile); | ||
} | ||
} | ||
Quinn-With-Two-Ns marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In terms of naming it would probably more consistent internally in the Java SDK to call this module
temporal-enviorment-configuration
, but that isn't consistent with the Go SDKThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 The
envconfig
shortened term for the modules/packages/namespaces myself is kinda nice