Add script module support for Python/C#/F#/Bash #385
Conversation
So that it can use `Octopus.Calamari.CopyWorkingDirectoryIncludingKeyTo`
There was a problem hiding this comment.
The python refactor looks good to me. I think we should consider a world where script modules have scripts for each syntax. Rob has made some progress doing this for script steps so I don't think it's unimaginable. There is nothing I can see to prevent a merge.
| @@ -15,6 +18,15 @@ public static string GetLibraryScriptModuleName(string variableName) | |||
| return variableName.Replace("Octopus.Script.Module[", "").TrimEnd(']'); | |||
| } | |||
|
|
|||
| public static ScriptSyntax GetLibraryScriptModuleLangauge(VariableDictionary variables, string variableName) | |||
There was a problem hiding this comment.
| public static ScriptSyntax GetLibraryScriptModuleLangauge(VariableDictionary variables, string variableName) | |
| public static ScriptSyntax GetLibraryScriptModuleLanguage(VariableDictionary variables, string variableName) |
| public static ScriptSyntax GetLibraryScriptModuleLangauge(VariableDictionary variables, string variableName) | ||
| { | ||
| var expectedName = variableName.Replace("Octopus.Script.Module[", "Octopus.Script.Module.Language["); | ||
| var syntaxVariable = variables.GetNames().FirstOrDefault(x => x == expectedName); |
There was a problem hiding this comment.
Throwing it out there: imagine a future where a module can contained a script for every syntax. What if we sent every syntax to Calamari and picked the syntax relevant to the script we are running.
There was a problem hiding this comment.
You know, I'm not convinced about that as an approach. (I know that's whats being talked about).
I'm really a fan of the capabilities approach, especially when we think about things like pre-preemptively showing a warning "your target needs to have powershell 5.1 on it", before executing a script, rather than during executing.
I also think the user has the most context on what script they want to run, and what language they want to use.
|
|
||
| static IEnumerable<string> PrepareScriptModules(VariableDictionary variables, string workingDirectory) | ||
| { | ||
| foreach (var variableName in variables.GetNames().Where(SpecialVariables.IsLibraryScriptModule)) |
There was a problem hiding this comment.
Should we log the modules that we are not loading?
There was a problem hiding this comment.
I was going to, but... imagine a world where we have 2 script modules (bash and f#) and 3 steps (bash, f# and powershell).
Do you really want to log a message in this scenario? TBH, I cant think of a nice way of handling the mismatch between languages....
| @@ -5,44 +5,34 @@ | |||
|
|
|||
| namespace Calamari.Integration.Scripting.Python | |||
| { | |||
| public class PythonScriptEngine : IScriptEngine | |||
| public class PythonScriptEngine : ScriptEngine | |||
There was a problem hiding this comment.
Thanks for converting this to inherit ScriptEngine.
| Script script, | ||
| CalamariVariableDictionary variables, | ||
| ICommandLineRunner commandLineRunner, | ||
| protected override IEnumerable<ScriptExecution> PrepareExecution(Script script, CalamariVariableDictionary variables, |
There was a problem hiding this comment.
IEnumerable<ScriptExecution> bothers me a bit because I am expecting just a single execution. I understand it's required to run the python dependency script. I don't have a good suggestion for an alternative.
There was a problem hiding this comment.
I wonder if we (eventually) need to refactor the python dependency install and script execution into the one call somehow. Food for thought.
There was a problem hiding this comment.
Either that, or we introduce another method for "preparing an execution for installing dependencies"?
We could just not override it on the other methods? 🤔
Stuff for the future.
…h lying around Relates to OctopusDeploy/Issues#3895 Fixes OctopusDeploy/Issues#5508
| @@ -124,13 +124,6 @@ function Import-ScriptModule([string]$moduleName, [string]$moduleFilePath) | |||
| Write-Warning "Failed to import Script Module '$moduleName'" | |||
| Throw | |||
| } | |||
| Finally | |||
There was a problem hiding this comment.
Related to OctopusDeploy/Issues#5508
Relates to OctopusDeploy/Issues#5499
Also fixes support with the Python bootstrapper so that it works with
Octopus.Calamari.CopyWorkingDirectoryIncludingKeyTo