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.

Sign up for GitHub

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

improve diagnose for invalide environment/command line arguments #19

Merged
merged 2 commits into from
Nov 22, 2024
Merged

improve diagnose for invalide environment/command line arguments #19

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34b18cd
improve diagnose for invalide environment/command line arguments
hduelme Nov 22, 2024
9bb3532
Bugfix: Do not ignore environment args when config file is used.
haumacher Nov 22, 2024
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
26 changes: 20 additions & 6 deletions mjsip-util/src/main/java/org/mjsip/config/OptionParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public static MetaConfig parseOptions(String[] args, String defaultConfigFile, O
}

CmdLineException problem = null;
boolean invalidValueGiven = false;
try {
// First try, use options form environment and command line.
Collection<String> arguments = new ArrayList<>(envArgs);
Expand All @@ -71,13 +72,18 @@ public static MetaConfig parseOptions(String[] args, String defaultConfigFile, O
} catch (CmdLineException ex) {
// Happens, when required configurations are not given on the command line.
problem = ex;
String message = ex.getMessage();
invalidValueGiven = message != null && message.contains(" is not a valid value for ");
if (invalidValueGiven) {
LOG.warn("{}. Reading from environment/command line aborted", message);
}
}

String argFile = metaConfig.configFile;

File file;
if (argFile != null && !argFile.isBlank()) {
if ("none".equals(argFile.toLowerCase())) {
if ("none".equalsIgnoreCase(argFile)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Gute Idee - wird das an anderen Stellen der Konfigurationsverarbeitung eigentlich auch so gemacht?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Aktuell wird in phoneblock recordings noch nur mit equals geprüft und die Methode getConfigFile aus MetaConfig ist auch noch nicht angepasst.

file = null;
} else {
file = new File(argFile);
Expand All @@ -87,7 +93,7 @@ public static MetaConfig parseOptions(String[] args, String defaultConfigFile, O
}
}
} else if (defaultConfigFile != null) {
String fileName = System.getProperty("user.home") + "/" + defaultConfigFile;
String fileName = System.getProperty("user.home") + File.pathSeparator + defaultConfigFile;
Copy link
Owner

Choose a reason for hiding this comment

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

Top! Was für ein Anfängerfehler von mir....

file = new File(fileName);
if (!file.exists()) {
file = null;
Expand All @@ -97,13 +103,21 @@ public static MetaConfig parseOptions(String[] args, String defaultConfigFile, O
}

if (file!= null) {
LOG.debug("Reading options from: " + file.getAbsolutePath());
if (invalidValueGiven) {
LOG.warn("Reading options from: {}", file.getAbsolutePath());
} else {
LOG.debug("Reading options from: {}", file.getAbsolutePath());
}
Copy link
Owner

Choose a reason for hiding this comment

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

Das eigentliche Problem ist aber wohl, dass beim zweiten Durchlauf, wenn die Konfigurationsdatei gelesen wird, die Umgebungsvariablen komplett ignoriert werden. Die Argumente werden wieder hinzugefügt, die Umgebungsvariablen aber nicht.


ConfigFile configFile = new ConfigFile(file);

// Parse all arguments again to check for required arguments not given but give
// precedence to arguments given on the command line.
// Parse all arguments again to check for required arguments not given and to
// allow overwriting configuration file arguments with environment and command
// line.
// Use the following precedence order: Arguments in the configuration file (lowest),
// arguments from the environment, arguments given on the command line (highest).
Collection<String> arguments = new ArrayList<>(configFile.toArguments());
arguments.addAll(envArgs);
arguments.addAll(Arrays.asList(args));

parser.parseArgument(arguments);
Expand Down Expand Up @@ -135,7 +149,7 @@ public static String getValueFromEnvironment(String optionName) {
String env = optionName.substring(2).replace('-', '_').toUpperCase();
String value = System.getenv(env);
if (value != null) {
LOG.info("Using '" + optionName + "' from environment: " + env + "=" + value);
LOG.info("Using '{}' from environment: {}={}", optionName, env, value);
}
return value;
}
Expand Down