Skip to content

Commit

Permalink
Miscellaenous bug fixes and tweaks: (#242)
Browse files Browse the repository at this point in the history
- Fixed bug where python installed using .msi on Windows would not work
- Improved robustness of deleting temporary files
- Fixed null reference exception in some cases in python runner
- Fixed variable name typo
- Remove unnecessary new line in config.json that was causing inconsistent line endings on windows
  • Loading branch information
ralph-msft authored Mar 20, 2024
1 parent 17d5b76 commit 3a79988
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ public static void CreateAiHubProjectConfigJsonFile(string subscription, string

var configJson = JsonSerializer.Serialize(configJsonData, new JsonSerializerOptions { WriteIndented = true });
var configJsonFile = new FileInfo("config.json");
FileHelpers.WriteAllText(configJsonFile.FullName, configJson + "\n", new UTF8Encoding(false));
FileHelpers.WriteAllText(configJsonFile.FullName, configJson, new UTF8Encoding(false));

Console.WriteLine($"{configJsonFile.Name} (saved at {configJsonFile.Directory})\n");
Console.WriteLine(" " + configJson.Replace("\n", "\n "));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public partial class AzCliConsoleGui
{
public static string GetSubscriptionUserName(string subscriptionId)
{
if (_subscriptionIdToUsreName.TryGetValue(subscriptionId, out var userName))
if (_subscriptionIdToUserName.TryGetValue(subscriptionId, out var userName))
{
return userName;
}
Expand Down Expand Up @@ -176,9 +176,9 @@ private static void DisplayNameAndId(AzCli.SubscriptionInfo subscription)

private static void CacheSubscriptionUserName(AzCli.SubscriptionInfo subscription)
{
_subscriptionIdToUsreName[subscription.Id] = subscription.UserName;
_subscriptionIdToUserName[subscription.Id] = subscription.UserName;
}

private static Dictionary<string, string> _subscriptionIdToUsreName = new Dictionary<string, string>();
private static Dictionary<string, string> _subscriptionIdToUserName = new Dictionary<string, string>();
}
}
12 changes: 5 additions & 7 deletions src/common/details/helpers/file_helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -793,13 +793,11 @@ public static void EnsureDirectoryForFileExists(string fileName)
if (IsStandardInputReference(fileName)) return;
if (IsStandardOutputReference(fileName)) return;

var s1 = fileName.LastIndexOf(Path.DirectorySeparatorChar);
var s2 = fileName.LastIndexOf(Path.AltDirectorySeparatorChar);
var sep = Math.Max(s1, s2);
if (sep <= 0) return;

var dir = fileName.Substring(0, sep);
if (!Directory.Exists(dir)) Directory.CreateDirectory(dir);
string dir = Path.GetDirectoryName(fileName);
if (!string.IsNullOrEmpty(dir) && !Directory.Exists(dir))
{
Directory.CreateDirectory(dir);
}
}

public static void CopyFile(string path1, string file1, string path2, string file2 = null, bool verbose = true)
Expand Down
22 changes: 15 additions & 7 deletions src/common/details/helpers/process_helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,19 @@ public static async Task<ProcessOutput> RunShellCommandAsync(string command, str
}
};

var process = TryCatchHelpers.TryCatchNoThrow<Process>(() => StartShellCommandProcess(command, arguments, addToEnvironment, redirectOutput), null, out Exception processException);
if (process == null)
Process process;
try
{
process = StartShellCommandProcess(command, arguments, addToEnvironment, redirectOutput);
}
catch (Exception processException)
{
SHELL_DEBUG_TRACE($"ERROR: {processException}");
return new ProcessOutput() { StdError = processException.ToString() };
return new ProcessOutput()
{
ExitCode = -1,
StdError = processException.ToString()
};
}

if (redirectOutput)
Expand All @@ -176,10 +184,10 @@ public static async Task<ProcessOutput> RunShellCommandAsync(string command, str
}

var output = new ProcessOutput();
output.StdOutput = process != null ? sbOut.ToString().Trim(' ', '\r', '\n') : "";
output.StdError = process != null ? sbErr.ToString().Trim(' ', '\r', '\n') : processException.ToString();
output.MergedOutput = process != null ? sbMerged.ToString().Trim(' ', '\r', '\n') : "";
output.ExitCode = process != null ? process.ExitCode : -1;
output.StdOutput = sbOut.ToString().Trim(' ', '\r', '\n');
output.StdError = sbErr.ToString().Trim(' ', '\r', '\n');
output.MergedOutput = sbMerged.ToString().Trim(' ', '\r', '\n');
output.ExitCode = process.ExitCode;

if (!string.IsNullOrEmpty(output.StdOutput)) SHELL_DEBUG_TRACE($"---\nSTDOUT\n---\n{output.StdOutput}");
if (!string.IsNullOrEmpty(output.StdError)) SHELL_DEBUG_TRACE($"---\nSTDERR\n---\n{output.StdError}");
Expand Down
39 changes: 29 additions & 10 deletions src/common/details/helpers/python_runner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,22 @@ public static async Task<ProcessOutput> RunPythonScriptAsync(string script, stri
return new ProcessOutput() { ExitCode = -1 };
}

var tempFile = Path.GetTempFileName() + ".py";
FileHelpers.WriteAllText(tempFile, script, new UTF8Encoding(false));
string tempFile = null;

try
{
tempFile = Path.GetTempFileName() + ".py";
FileHelpers.WriteAllText(tempFile, script, new UTF8Encoding(false));

args = args != null
? $"\"{tempFile}\" {args}"
: $"\"{tempFile}\"";
return await ProcessHelpers.RunShellCommandAsync(_pythonBinary, args, addToEnvironment, stdOutHandler, stdErrHandler, mergedOutputHandler);
}
finally
{
File.Delete(tempFile);
if (tempFile != null)
File.Delete(tempFile);
}
}

Expand Down Expand Up @@ -72,8 +75,8 @@ public static string RunEmbeddedPythonScript(ICommandValues values, string scrip
{
AI.DBG_TRACE_WARNING($"RunEmbeddedPythonScript: exit={exit}");

output = output.Trim('\r', '\n', ' ');
output = "\n\n " + output.Replace("\n", "\n ");
output = output?.Trim('\r', '\n', ' ');
output = "\n\n " + output?.Replace("\n", "\n ");

var info = new List<string>();

Expand Down Expand Up @@ -178,7 +181,7 @@ public static string RunEmbeddedPythonScript(ICommandValues values, string scrip

private static string ParseOutputAndSkipLinesUntilStartsWith(string output, string startsWith)
{
var lines = output.Split('\n');
var lines = output.Split(new[] { "\r\n", "\n", "\r" }, StringSplitOptions.None);
var sb = new StringBuilder();
var skip = true;
foreach (var line in lines)
Expand All @@ -200,17 +203,33 @@ private static string EnsureFindPython()
if (_pythonBinary == null)
{
_pythonBinary = FindPython();
AI.DBG_TRACE_VERBOSE($"Python found: {_pythonBinary}");
}

return _pythonBinary;
}

private static string FindPython()
{
var lastTry = FindPythonBinaryInOsPath();
var process = ProcessHelpers.RunShellCommandAsync(lastTry, "--version").Result;
if (process.ExitCode == 0 && process.MergedOutput.Contains("Python 3.")) return lastTry;
string fullPath = FindPythonBinaryInOsPath();
string pythonExec = fullPath;
if (OperatingSystem.IsWindows())
{
// TODO FIXME Longer term we really shouldn't be wrapping calls to python in cmd /c python
// and instead just calling python directly

// since we found the python executable in our standard search path, we can skip passing
// the entire path since it may contain spaces (e.g. C:\Program Files\Python312\python.exe)
// which can cause irritating errors requiring complex escaping. Instead, we can just pass
// the executable name and let Windows will handle querying the OS search path for us
pythonExec = Path.GetFileName(fullPath);
}

var process = ProcessHelpers.RunShellCommandAsync(pythonExec, "--version").Result;
if (process.ExitCode == 0 && process.MergedOutput.Contains("Python 3."))
{
AI.DBG_TRACE_VERBOSE($"Python found: {fullPath}");
return pythonExec;
}

return null;
}
Expand Down

0 comments on commit 3a79988

Please sign in to comment.