SqlScript: DisableVariables from Invoke-SQLCmd not available

[julfur]

Hi

I've been trying to use the SqlScript to run a T-SQL that basically create an SQL Agent Job to schedule Ola's Maintenance stored procedures.

Ola's default logging behavior use this output for each step :

@output_file_name=N'D:\Microsoft SQL Server\Maintenance\$(ESCAPE_SQUOTE(JOBNAME))_$(ESCAPE_SQUOTE(STEPID))_$(ESCAPE_SQUOTE(DATE))_$(ESCAPE_SQUOTE(TIME)).txt'

As the script is executed by a DSC SqlScript block, the following error is thrown :

PowerShell DSC resource MSFT_SqlScript  failed to execute Set-TargetResource functionality with error message: 'JOBNAME' scripting variable not defined
    + CategoryInfo          : InvalidOperation: (:) [], CimException
    + FullyQualifiedErrorId : ProviderOperationExecutionFailure

The internal SQL variable JOBNAME is detected as a PowerShell variable and DSC is asking to supply that variable.

I know that an option is available with Invoke-SQLCmd to prevent that, DisableVariables.
Is there anything similar that would have not been documented I could use to keep my output_file_name ?

Thanks for your help,
Julfur

[johlju]

The cmdlet Invoke-Sqlcmd is used by the SqlScript resource, so it would be possible to extend the resource with this parameter, and make sure it can pass it to this helper function.

https://github.com/PowerShell/SqlServerDsc/blob/2337c7cbaa9c47d5bf82802ec77167922ec892b6/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1#L2334-L2386

Should be pretty straightforward change. Happy to review a PR for this!

[Kreby]

@johlju I currently need this feature for a configuration I'm working on. It's been a while since I've used github but I'd like to submit a PR to add this if you don't mind helping me should I run into any issues.

I have a few initial questions about the scope/context of this change.

1. Can you think of any other DSC resources that would need to be update other than the SqlScript and SqlScriptQuery?
2. Looking through the existing integration/unit tests for SqlScript and SqlScriptQuery I don't currently see explicit tests for other optional parameters like "Variables" but I do see that it is being used in other tests. I assume you'd like to see both a unit and integration tests for this feature?

[johlju]

@Kreby happy to review a PR for this and happily help with any issues. Due to my day job it can take a while for me to respond though.

Can you think of any other DSC resources that would need to be update other than the SqlScript and SqlScriptQuery?

A quick search on GitHub (see link) it looks like it is the only two resources that is using the helper function.
https://github.com/dsccommunity/SqlServerDsc/search?q=Invoke-SqlScript&unscoped_q=Invoke-SqlScript

Looking through the existing integration/unit tests for SqlScript and SqlScriptQuery I don't currently see explicit tests for other optional parameters like "Variables" but I do see that it is being used in other tests. I assume you'd like to see both a unit and integration tests for this feature?

Looking here I see the the resource is fully covered using tests.
https://codecov.io/gh/dsccommunity/SqlServerDsc/src/master/source/DSCResources/DSC_SqlScript/DSC_SqlScript.psm1

Yes, I would like to see at least a unit tests for this, although the existing unit tests for the DSC resources and the unit tests for the helper function might cover this change as well when I look how it was coded.
It looks like all parameters (bound or not) are passed to Invoke-SqlScript. Then all parameters are passed to the Invoke-SqlCmd. If the DSC configuration did not contain Variables then it would pass $null to Invoke-SqlCmd and it seems Invoke-SqlCmd ignores that. So if DisableVariables behave the same then you should not need to change or add to the unit tests. 🤔

For some resources we can't make integration tests due to limitations in the build worker. Although in this case I would like to see an integration test as it helps be verify functionality (during review) for this PR and future PR's. 🙂 There are existing integration tests that uses the Variables parameter so that will make sure existing functionality does not break, so you just need to add another integration tests to test this new parameter.

I also would like to see an example added to the resource example folder.

[Kreby]

@johlju that all sounds good to me. I'll make sure to include an integration test and an example. I'll have to think about what would work as a good simple test/example 🤔.

Looking through the existing tests it does raise a question about the configuration data.
https://github.com/dsccommunity/SqlServerDsc/blob/master/tests/Integration/DSC_SqlScript.config.ps1#L36-L53

I'd need to confirm the behavior first but I'd suspect that using both Variables and DisableVariables would lead to some complications. How would you prefer I work around that in the configuration data? I see different databases listed a Database1Name and Database2Name so should I add 3 additional variables with different scripts that the new test would then use or something else?

It might take me a little bit to get up to speed on the tests but I'm going to try to find some time to get started hopefully by the end of the week.

[johlju]

You should add scripts for these tests as appropriate, but it is optional to add the new properties or scripts to the configuration data. You can "hard-code" them directly in the configuration and integration test(s) if you like. Often we use the values in the configuration data to get the expected values when testing output from Get-DscConfiguration. If a value need to change we just need to change it in one place. But that happens so rarely. So use the configuration data block if you like or as you see fit.