MSFT_SqlAgentOperator: Added resource for SQL Server Agent Operators #1255

jpomfret · 2018-11-21T16:37:49Z

Pull Request (PR) description

Added resource for Operators, can currently add an operator with an email address.

I've also added tests which I'm having some issues with, I have got the mock working for all but the throwing error pieces, I'm submitting this PR in the hope someone can point me in the right direction on that.

Task list

Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
Entry should say what was changed, and how that affects users (if applicable).
Resource documentation added/updated in README.md.
Resource parameter descriptions added/updated in README.md, schema.mof
and comment-based help.
Comment-based help added/updated.
Localization strings added/updated in all localization files as appropriate.
Examples appropriately added/updated.
Unit tests added/updated. See DSC Resource Testing Guidelines.
Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is

johlju · 2018-12-04T15:16:31Z

@jpomfret Thank you for this PR. I have been busy with my day job so haven't been able to do reviews as much as I would have wanted. But slowly being able to get back into this. I will get to this PR as soon as possible.

jpomfret · 2018-12-14T18:27:22Z

Thanks and not a problem. I believe my issues are with my mocking rather than the DSC Resource but I've been unable to figure out why the methods aren't working.

codecov-io · 2018-12-21T16:48:46Z

Codecov Report

Merging #1255 into dev will decrease coverage by <1%.
The diff coverage is 85%.

@@         Coverage Diff          @@
##            dev   #1255   +/-   ##
====================================
- Coverage    97%     97%   -1%     
====================================
  Files        34      35    +1     
  Lines      4231    4311   +80     
====================================
+ Hits       4140    4208   +68     
- Misses       91     103   +12

johlju · 2019-01-04T10:58:57Z

Thanks for sending this in, sorry for not having review this until now. I will review this PR now, and see if I can see what's going on with the tests.

johlju

Awesome work on this. I haven't reviewed the tests yet, I will do that when these changes has been made since they will affect the test code too.

Reviewed 5 of 7 files at r1, 2 of 3 files at r2.
Reviewable status: 7 of 8 files reviewed, 37 unresolved discussions (waiting on @jpomfret)

a discussion (no related file):
Could you please rebase against branch dev using git rebase (not by using git pull or git merge, to keep the commit history). If you don't know how to rebase your local dev and working branch, please look at how to Resolve merge conflicts.
Let me know if you need any assistance.

a discussion (no related file):
Please add localization to this resource, see https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#localization, and you can look at the SqlServerDatabaseMail resource.

README.md, line 322 at r2 (raw file):

#### Parameters

Missing the Ensure parameter in the list.

README.md, line 324 at r2 (raw file):

The SQL Agent Operator name.

Please make sure that the descriptions in the schema.mof and the README.md (and comment-based help) match.

Throughout the parameters.

README.md, line 336 at r2 (raw file):

SQlAgentOperator

Typo in the link. It should be 'SqlAgentOperator', lower-case 'q'.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 18 at r2 (raw file):

.PARAMETER ...

Missing EmailAddress in the comment-based help.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 31 at r2 (raw file):

$Ensure

Non-mandatory parameters should not be needed in the Get-TargetResource function (should of course still be in the Set- and Test-functions).
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#get-targetresource-should-not-contain-unused-non-mandatory-parameters

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 38 at r2 (raw file):

[Parameter(Mandatory = $true)]

This should not mandatory if changed to the type qualifier Write. See comment in the schema.mof.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 54 at r2 (raw file):

Connect-SQL -SQLServer $ServerName -SQLInstanceName $InstanceName

A recently merged PR changed the parameter names for this helper function, please revise. Throughout the code.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 56 at r2 (raw file):

if ($sqlServerObject)

If this fails, an error should be thrown instead of silently return an empty hashtable.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 60 at r2 (raw file):

$(

We should not need the $(...) surrounding this line?

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 64 at r2 (raw file):

$Ensure = 'Present'

When the non-mandatory parameter Ensure is removed, then this variable should be written in lower-case letter (local variable), $ensure.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 71 at r2 (raw file):

$SqlOperatorEmail = $EmailAddress

If the object was not found, the properties that cannot be set to the actual value in the hash table should be added as null values as that is the actual value (it does not exist). See example how this can be done; https://github.com/PowerShell/SqlServerDsc/blob/5757fdbb425988f8063b43bf41da4012c352552e/DSCResources/MSFT_SqlServerDatabaseMail/MSFT_SqlServerDatabaseMail.psm1#L66-L79

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 76 at r2 (raw file):

$Name

I think we should use the name property from the $sqlOperatorObject object instead of returning the parameter.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 80 at r2 (raw file):

$SqlOperatorEmail

I think we should change this to return $sqlOperatorObject.EmailAddress.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 100 at r2 (raw file):

.PARAMETER ...

Missing EmailAddress in the comment-based help.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 137 at r2 (raw file):

if ($sqlServerObject)

If this fails, an error should be thrown instead of silently return an empty hashtable.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 147 at r2 (raw file):

if ($PSBoundParameters.ContainsKey('EmailAddress')) {

We should have the open brace on a separate row by its own.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 150 at r2 (raw file):

Write-Verbose -Message "Updating SQL Agent Operator $Name with specified settings."

We should wrote out what property we are changing and to what value.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 157 at r2 (raw file):

throw New-TerminatingError

These should be changed to use the new localized variants, see previous comment. Throughout.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 173 at r2 (raw file):

if ($PSBoundParameters.ContainsKey('EmailAddress')) {

We should have the open brace on a separate row by its own.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 177 at r2 (raw file):

New-VerboseMessage -Message "Created SQL Agent Operator $Name."

This verbose message should be moved under next row.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 200 at r2 (raw file):

Dropped

I think we should use 'Deleted' here instead, since that word is used above in the previous message.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 229 at r2 (raw file):

.PARAMETER ...

Missing EmailAddress in the comment-based help.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 276 at r2 (raw file):

"Ensure is set to Absent. The SQL Agent Operator $Name should be dropped"

Maybe we should also say that an operator was found, and it should not exist, so it is being deleted.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 276 at r2 (raw file):

dropped

Maybe we should use the word 'deleted' in the verbose message here since that is used elsewhere?

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 285 at r2 (raw file):

 "Ensure is set to Present. The SQL Agent Operator $Name should be created"

See if we can revise this verbose message to? See previous comment. I'm I wrong in thinking that 'Ensure is set to Present.' is unnecessary here (or that information could be moved to the top of the Test-function)?

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 288 at r2 (raw file):

$PSBoundParameters.ContainsKey('EmailAddress')

We should move this as the first condition, we should not evaluate the other condition if this condition is false.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 290 at r2 (raw file):

"SQL Agent Operator $Name exists but has the wrong email address"

I think we should write out what value EmailAddress has and what the expected value is.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.schema.mof, line 6 at r2 (raw file):

ServerName

Not sure we need to have ServerName as Key since it defaults to $env:COMPUTERNAME'? It should be changed to Write`, unless you know of an problem not having it as a Key.

If changed to Write, then please change the description to contain a mention of the default value, e.g. 'Defaults to $env:COMPUTERNAME'. Please make this change here, in the README.md, and the comment-based help.

DSCResources/MSFT_SqlAgentOperator/en-US/MSFT_SqlAgentOperator.strings.psd1, line 1 at r2 (raw file):

# Localized resources for SqlServerRole

This file is currently not used, see my other comment about localization so this file will be used correctly.

Examples/Resources/SqlAgentOperator/1-AddOperator.ps1, line 2 at r2 (raw file):

<#
.EXAMPLE

Please indent the comment based help on step more.

<#
    .EXAMPLE
        This example shows how to ensure that the SQL Agent Operator
        DbaTeam exists with the correct email address.
#>

Examples/Resources/SqlAgentOperator/1-AddOperator.ps1, line 9 at r2 (raw file):

param(

We shoudl have the open paranthesis on a separate row.

Examples/Resources/SqlAgentOperator/1-AddOperator.ps1, line 12 at r2 (raw file):

$SqlAdministratorCredential

This parameter is not used in the example.

Examples/Resources/SqlAgentOperator/2-RemoveOperator.ps1, line 2 at r2 (raw file):

<#
.EXAMPLE

Please indent the comment based help on step more.

Examples/Resources/SqlAgentOperator/2-RemoveOperator.ps1, line 9 at r2 (raw file):

param(

We shoudl have the open paranthesis on a separate row.

Examples/Resources/SqlAgentOperator/2-RemoveOperator.ps1, line 12 at r2 (raw file):

$SqlAdministratorCredential

This parameter is not used in the example.

jpomfret

Reviewable status: 0 of 8 files reviewed, 37 unresolved discussions (waiting on @johlju)

a discussion (no related file):

Previously, johlju (Johan Ljunggren) wrote…

Could you please rebase against branch dev using git rebase (not by using git pull or git merge, to keep the commit history). If you don't know how to rebase your local dev and working branch, please look at how to Resolve merge conflicts.
Let me know if you need any assistance.

Followed the 'Resolve merge conflicts' document. Done.

a discussion (no related file):

Previously, johlju (Johan Ljunggren) wrote…

Please add localization to this resource, see https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#localization, and you can look at the SqlServerDatabaseMail resource.

This is done, let me know if you'd change any of the messaging.

README.md, line 322 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

#### Parameters
Missing the Ensure parameter in the list.

Done.

README.md, line 324 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

The SQL Agent Operator name.
Please make sure that the descriptions in the schema.mof and the README.md (and comment-based help) match.

Throughout the parameters.

done

README.md, line 336 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

SQlAgentOperator
Typo in the link. It should be 'SqlAgentOperator', lower-case 'q'.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 18 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

.PARAMETER ...
Missing EmailAddress in the comment-based help.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 31 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$Ensure
Non-mandatory parameters should not be needed in the Get-TargetResource function (should of course still be in the Set- and Test-functions).
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#get-targetresource-should-not-contain-unused-non-mandatory-parameters

Done. I just removed this from the parameters, it wasn't actually used in the function.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 38 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

[Parameter(Mandatory = $true)]
This should not mandatory if changed to the type qualifier Write. See comment in the schema.mof.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 54 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Connect-SQL -SQLServer $ServerName -SQLInstanceName $InstanceName
A recently merged PR changed the parameter names for this helper function, please revise. Throughout the code.

done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 56 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

if ($sqlServerObject)
If this fails, an error should be thrown instead of silently return an empty hashtable.

Done. I used a terminating error with the Error Category ConnectionError, let me know if that is not appropriate.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 60 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$(
We should not need the $(...) surrounding this line?

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 64 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$Ensure = 'Present'
When the non-mandatory parameter Ensure is removed, then this variable should be written in lower-case letter (local variable), $ensure.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 71 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$SqlOperatorEmail = $EmailAddress
If the object was not found, the properties that cannot be set to the actual value in the hash table should be added as null values as that is the actual value (it does not exist). See example how this can be done; https://github.com/PowerShell/SqlServerDsc/blob/5757fdbb425988f8063b43bf41da4012c352552e/DSCResources/MSFT_SqlServerDatabaseMail/MSFT_SqlServerDatabaseMail.psm1#L66-L79

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 76 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$Name
I think we should use the name property from the $sqlOperatorObject object instead of returning the parameter.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 80 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$SqlOperatorEmail
I think we should change this to return $sqlOperatorObject.EmailAddress.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 100 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

.PARAMETER ...
Missing EmailAddress in the comment-based help.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 137 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

if ($sqlServerObject)
If this fails, an error should be thrown instead of silently return an empty hashtable.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 147 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

if ($PSBoundParameters.ContainsKey('EmailAddress')) {
We should have the open brace on a separate row by its own.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 150 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Write-Verbose -Message "Updating SQL Agent Operator $Name with specified settings."
We should wrote out what property we are changing and to what value.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 157 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

throw New-TerminatingError
These should be changed to use the new localized variants, see previous comment. Throughout.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 173 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

if ($PSBoundParameters.ContainsKey('EmailAddress')) {
We should have the open brace on a separate row by its own.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 177 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

New-VerboseMessage -Message "Created SQL Agent Operator $Name."
This verbose message should be moved under next row.

I removed this message as I felt like I had some duplicate messaging, let me know what you think, I can re-add if you prefer.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 200 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Dropped
I think we should use 'Deleted' here instead, since that word is used above in the previous message.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 229 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

.PARAMETER ...
Missing EmailAddress in the comment-based help.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 276 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

"Ensure is set to Absent. The SQL Agent Operator $Name should be dropped"
Maybe we should also say that an operator was found, and it should not exist, so it is being deleted.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 276 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

dropped
Maybe we should use the word 'deleted' in the verbose message here since that is used elsewhere?

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 285 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

 "Ensure is set to Present. The SQL Agent Operator $Name should be created"
See if we can revise this verbose message to? See previous comment. I'm I wrong in thinking that 'Ensure is set to Present.' is unnecessary here (or that information could be moved to the top of the Test-function)?

I revised these messages but left in absent/present. Are you thinking it would be better to just have a verbose message at the beginning that states whether ensure is present or absent?

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 288 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$PSBoundParameters.ContainsKey('EmailAddress')
We should move this as the first condition, we should not evaluate the other condition if this condition is false.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 290 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

"SQL Agent Operator $Name exists but has the wrong email address"
I think we should write out what value EmailAddress has and what the expected value is.

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.schema.mof, line 6 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

ServerName
Not sure we need to have ServerName as Key since it defaults to $env:COMPUTERNAME'? It should be changed to Write`, unless you know of an problem not having it as a Key.

If changed to Write, then please change the description to contain a mention of the default value, e.g. 'Defaults to $env:COMPUTERNAME'. Please make this change here, in the README.md, and the comment-based help.

Done.

DSCResources/MSFT_SqlAgentOperator/en-US/MSFT_SqlAgentOperator.strings.psd1, line 1 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

This file is currently not used, see my other comment about localization so this file will be used correctly.

Done.

Examples/Resources/SqlAgentOperator/1-AddOperator.ps1, line 2 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Please indent the comment based help on step more.

<#
    .EXAMPLE
        This example shows how to ensure that the SQL Agent Operator
        DbaTeam exists with the correct email address.
#>

Done.

Examples/Resources/SqlAgentOperator/1-AddOperator.ps1, line 9 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

param(
We shoudl have the open paranthesis on a separate row.

Done.

Examples/Resources/SqlAgentOperator/1-AddOperator.ps1, line 12 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$SqlAdministratorCredential
This parameter is not used in the example.

Done.

Examples/Resources/SqlAgentOperator/2-RemoveOperator.ps1, line 2 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Please indent the comment based help on step more.

Done.

Examples/Resources/SqlAgentOperator/2-RemoveOperator.ps1, line 9 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

param(
We shoudl have the open paranthesis on a separate row.

Done.

Examples/Resources/SqlAgentOperator/2-RemoveOperator.ps1, line 12 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$SqlAdministratorCredential
This parameter is not used in the example.

Done.

jpomfret

Reviewed 1 of 7 files at r1.
Reviewable status: 0 of 8 files reviewed, 37 unresolved discussions (waiting on @johlju and @jpomfret)

jpomfret

Reviewed 1 of 3 files at r2.
Reviewable status: 0 of 8 files reviewed, 37 unresolved discussions (waiting on @johlju and @jpomfret)

jpomfret

Reviewed 8 of 8 files at r3.
Reviewable status: all files reviewed, 37 unresolved discussions (waiting on @johlju)

stale · 2019-01-22T11:52:22Z

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

jpomfret · 2019-01-22T13:48:48Z

@johlju - I don't want this to go stale, I have reviewed all your changes. My issue is still with the tests. I have tried again to get the mocking to work for the drop operator but am struggling.

mdaniou · 2019-01-22T15:04:23Z

@jpomfret I feel your pain, this is the same for me.
I'll see if I can help you with that.

mdaniou · 2019-01-23T07:52:42Z

@jpomfret This is only a guess but you might want to have a look at the integration test MSFT_SqlSetup.Integration.Tests.ps1

Line 170, the Before All block defines the ResourceId that is used later on the It blocks

BeforeAll { $resourceId = "[$($script:DSCResourceFriendlyName)]Integration_Test" }

Line 249
It 'Should have set the resource and all the parameters should match' { $resourceCurrentState = $script:currentConfiguration | Where-Object -FilterScript { $_.ConfigurationName -eq $configurationName } | Where-Object -FilterScript { $_.ResourceId -eq $resourceId }

My guess is that somehow your It block in error returns 'Absent' because the wrong occurrence of the configuration is used.
Do you think that this could be your problem and inserting this logic would fix your integration test ?

mdaniou

Reviewable status: all files reviewed, 39 unresolved discussions (waiting on @johlju and @jpomfret)

README.md, line 327 at r3 (raw file):

* **`[String]` Ensure** _(Write)_: Specifies if the SQL Agent Operator should
  be present or absent. Default is Present. { *Present* | Absent }
* **`[String]` ServerName** _(Key)_: The host name of the SQL Server to be configured. Default is $env:COMPUTERNAME.

to fix the error :
C:\projects\sqlserverdsc\README.md: 327: MD013/line-length Line length [Expected: 80; Actual: 116]

Ensure that this line is no longer than 80 characters :

[String] ServerName (Key): The host name of the SQL Server to be
configured. Default is $env:COMPUTERNAME.

README.md, line 329 at r3 (raw file):

* **`[String]` ServerName** _(Key)_: The host name of the SQL Server to be configured. Default is $env:COMPUTERNAME.
* **`[String]` InstanceName** _(Key)_: The name of the SQL instance to be configured.
* **`[String]` EmailAddress** _(Write)_: The email address to be used for the SQL Agent Operator.

Same here :

[String] EmailAddress (Write): The email address to be used for
the SQL Agent Operator.

jpomfret

Reviewable status: all files reviewed, 39 unresolved discussions (waiting on @johlju and @mdaniou)

README.md, line 327 at r3 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

to fix the error :
C:\projects\sqlserverdsc\README.md: 327: MD013/line-length Line length [Expected: 80; Actual: 116]

Ensure that this line is no longer than 80 characters :

[String] ServerName (Key): The host name of the SQL Server to be
configured. Default is $env:COMPUTERNAME.

Done.

README.md, line 329 at r3 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Same here :

[String] EmailAddress (Write): The email address to be used for
the SQL Agent Operator.

Done.

johlju · 2019-01-27T11:58:29Z

@jpomfret I made a really terrible mistake - I force pushed over you recent changes, I'm so sorry 😞 Please, can you force push your local repo `git push my SqlAgentOperator' so we get back to the current state you are in? 'my' is the remote name to your fork.

johlju · 2019-01-27T11:59:06Z

I meant it should have said `git push my SqlAgentOperator --force'

jpomfret · 2019-01-27T12:08:46Z

No problem 😄. I just ran that and it looks like my latest changes (readme, and renaming the integration tests file) are in there.

johlju · 2019-01-27T12:10:45Z

@jpomfret Thank you 😌 🙇 This was one of those mistake one only do once and always remember 😄

johlju

Looking good. Just waiting for the tests to pass.

Reviewed 1 of 6 files at r8, 2 of 2 files at r9.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mdaniou)

johlju

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @jpomfret and @mdaniou)

README.md, line 323 at r10 (raw file):

#### Parameters
* **`[String]` Name** _(Key)_: The name of the SQL Agent Operator.

We should have a blank line before this one.
https://ci.appveyor.com/project/PowerShell/sqlserverdsc/builds/21918379/job/8tt3n4yphrfkvojb?fullLog=true#L2129

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 290 at r10 (raw file):

EmailAddress   = $EmailAddress

This parameter is no longer part of the Get-function.
https://ci.appveyor.com/project/PowerShell/sqlserverdsc/builds/21918379/job/8tt3n4yphrfkvojb?fullLog=true#L2872

Tests/Unit/MSFT_SqlAgentOperator.Tests.ps1, line 140 at r10 (raw file):

EmailAddress = '[email protected]'

E-mail address is not part of Get-function no longer. Throughout.
https://ci.appveyor.com/project/PowerShell/sqlserverdsc/builds/21918379/job/8tt3n4yphrfkvojb?fullLog=true#L2842

Tests/Unit/MSFT_SqlAgentOperator.Tests.ps1, line 289 at r10 (raw file):

        }

        Describe "MSFT_SqlAgentOperator\Set-TargetResource" -Tag 'Set' {

There are failing tests in Set-function
https://ci.appveyor.com/project/PowerShell/sqlserverdsc/builds/21918379/job/8tt3n4yphrfkvojb?fullLog=true#L2932

johlju · 2019-01-27T12:32:59Z

Just some minor problems with test failing - when the tests are passing this is good to be merged!

jpomfret

Reviewed 3 of 6 files at r8, 2 of 2 files at r9.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @johlju, @jpomfret, and @mdaniou)

README.md, line 323 at r10 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

We should have a blank line before this one.
https://ci.appveyor.com/project/PowerShell/sqlserverdsc/builds/21918379/job/8tt3n4yphrfkvojb?fullLog=true#L2129

Done.

DSCResources/MSFT_SqlAgentOperator/MSFT_SqlAgentOperator.psm1, line 290 at r10 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

EmailAddress   = $EmailAddress
This parameter is no longer part of the Get-function.
https://ci.appveyor.com/project/PowerShell/sqlserverdsc/builds/21918379/job/8tt3n4yphrfkvojb?fullLog=true#L2872

Done.

Tests/Unit/MSFT_SqlAgentOperator.Tests.ps1, line 140 at r10 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

EmailAddress = '[email protected]'
E-mail address is not part of Get-function no longer. Throughout.
https://ci.appveyor.com/project/PowerShell/sqlserverdsc/builds/21918379/job/8tt3n4yphrfkvojb?fullLog=true#L2842

Done.

jpomfret · 2019-01-27T12:55:49Z

I need to take a look at the set tests, they are throwing the wrong errors. I'll try and get back to it this afternoon (EST), thanks for your help.

jpomfret

Reviewed 3 of 3 files at r11.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @johlju and @mdaniou)

Tests/Unit/MSFT_SqlAgentOperator.Tests.ps1, line 289 at r10 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

There are failing tests in Set-function
https://ci.appveyor.com/project/PowerShell/sqlserverdsc/builds/21918379/job/8tt3n4yphrfkvojb?fullLog=true#L2932

Done. Changed the errors in the function, looked at database mail tests and fixed now.

…verDsc into SqlAgentOperator

jpomfret

Reviewable status: 10 of 11 files reviewed, 7 unresolved discussions (waiting on @johlju, @jpomfret, and @mdaniou)

Tests/Unit/MSFT_SqlAgentOperator.Tests.ps1, line 158 at r11 (raw file):

            }

            Context 'When the system is in the desired state for a sql agent operator' {

I'm not sure why these are failing, they work find on my local machine. Did I overwrite something AppVeyor needed?
https://ci.appveyor.com/project/PowerShell/sqlserverdsc/builds/21924958/job/49eq2ml3c6jyv03d?fullLog=true#L2847

johlju

Reviewable status: 10 of 11 files reviewed, 8 unresolved discussions (waiting on @johlju, @jpomfret, and @mdaniou)

Tests/Unit/MSFT_SqlAgentOperator.Tests.ps1, line 81 at r12 (raw file):

New-Object -TypeName Microsoft.SqlServer.Management.Smo.Agent.Operator

The tests are failing because this type does not exist when running the tests in AppVeyor. You need to "mock" this too, changing this to Object will make the tests pass. But then you have to resolve other tests that started to fail after this change (when I run the tests).

Tests/Unit/MSFT_SqlAgentOperator.Tests.ps1, line 154 at r12 (raw file):

Assert-MockCalled Connect-SQL -Exactly -Times 2 -Scope Context

We should use named parameters on all calls. This one should have -CommandName. Throughout all Assert-MockCalled.

Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 2 -Scope Context

jpomfret

Reviewed 1 of 1 files at r12.
Reviewable status: 10 of 11 files reviewed, 9 unresolved discussions (waiting on @johlju, @jpomfret, and @mdaniou)

Tests/Unit/MSFT_SqlAgentOperator.Tests.ps1, line 81 at r12 (raw file):

New-Object -TypeName Microsoft.SqlServer.Management.Smo.Agent.Opera

Tests/Unit/MSFT_SqlAgentOperator.Tests.ps1, line 81 at r12 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

New-Object -TypeName Microsoft.SqlServer.Management.Smo.Agent.Operator
The tests are failing because this type does not exist when running the tests in AppVeyor. You need to "mock" this too, changing this to Object will make the tests pass. But then you have to resolve other tests that started to fail after this change (when I run the tests).

Done.

Tests/Unit/MSFT_SqlAgentOperator.Tests.ps1, line 154 at r12 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Assert-MockCalled Connect-SQL -Exactly -Times 2 -Scope Context
We should use named parameters on all calls. This one should have -CommandName. Throughout all Assert-MockCalled.
Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 2 -Scope Context

Done.

jpomfret

Reviewed 1 of 1 files at r13.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @johlju and @mdaniou)

johlju

Reviewed 2 of 3 files at r11, 1 of 1 files at r12, 1 of 1 files at r13.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @mdaniou)

johlju

Reviewed 2 of 2 files at r14.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @mdaniou)

johlju · 2019-01-29T18:43:06Z

Really awesome work on this!

jpomfret · 2019-01-29T18:52:38Z

Thanks @johlju for all your reviews and help!

johlju · 2019-01-29T19:15:06Z

Thank you! Hope to see you again in another PR. 😄

johlju added the needs review The pull request needs a code review. label Dec 4, 2018

johlju requested changes Jan 4, 2019

View reviewed changes

johlju added waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. and removed needs review The pull request needs a code review. labels Jan 4, 2019

jpomfret force-pushed the SqlAgentOperator branch from 022ee1f to 9529367 Compare January 4, 2019 15:27

jpomfret commented Jan 7, 2019

View reviewed changes

stale bot added the abandoned The pull request has been abandoned. label Jan 22, 2019

mdaniou reviewed Jan 23, 2019

View reviewed changes

jpomfret commented Jan 23, 2019

View reviewed changes

stale bot removed the abandoned The pull request has been abandoned. label Jan 23, 2019

jpomfret added 9 commits January 23, 2019 11:09

add SqlAgentOperator resource

1f5c372

start of tests

98fbb7e

code fixes. change/check email only when passed in

e10b1cb

working on unit tests

c67c82f

removed write-host used for debugging tests

e6e25ea

Cleaned up test file

f0d971a

added note to Changelog

849778d

add docs

e325090

add examples

f0defb6

johlju force-pushed the SqlAgentOperator branch from ea45848 to fb2a4cc Compare January 27, 2019 11:53

jpomfret force-pushed the SqlAgentOperator branch from fb2a4cc to ea45848 Compare January 27, 2019 12:06

johlju approved these changes Jan 27, 2019

View reviewed changes

johlju requested changes Jan 27, 2019

View reviewed changes

jpomfret commented Jan 27, 2019

View reviewed changes

hopefully the last few fixes...

67e68e1

jpomfret commented Jan 27, 2019

View reviewed changes

jpomfret added 3 commits January 27, 2019 14:38

fix for absent test

90dae65

fix for absent test

0da2c20

Merge branch 'SqlAgentOperator' of https://github.com/jpomfret/SqlSer…

ef32daa

…verDsc into SqlAgentOperator

jpomfret commented Jan 27, 2019

View reviewed changes

johlju requested changes Jan 28, 2019

View reviewed changes

mocking the agent.operator

51188c7

jpomfret commented Jan 29, 2019

View reviewed changes

johlju reviewed Jan 29, 2019

View reviewed changes

Merge branch 'dev' into SqlAgentOperator

3876e66

johlju approved these changes Jan 29, 2019

View reviewed changes

johlju merged commit 9665e1c into dsccommunity:dev Jan 29, 2019

johlju removed the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Jan 29, 2019

jpomfret deleted the SqlAgentOperator branch January 29, 2019 18:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MSFT_SqlAgentOperator: Added resource for SQL Server Agent Operators #1255

MSFT_SqlAgentOperator: Added resource for SQL Server Agent Operators #1255

jpomfret commented Nov 21, 2018 •

edited by joeyaiello

Loading

johlju commented Dec 4, 2018

jpomfret commented Dec 14, 2018

codecov-io commented Dec 21, 2018 •

edited

Loading

johlju commented Jan 4, 2019

johlju left a comment

jpomfret left a comment

jpomfret left a comment

jpomfret left a comment

jpomfret left a comment

stale bot commented Jan 22, 2019

jpomfret commented Jan 22, 2019

mdaniou commented Jan 22, 2019

mdaniou commented Jan 23, 2019

mdaniou left a comment

jpomfret left a comment

johlju commented Jan 27, 2019

johlju commented Jan 27, 2019 •

edited

Loading

jpomfret commented Jan 27, 2019

johlju commented Jan 27, 2019 •

edited

Loading

johlju left a comment

johlju left a comment

johlju commented Jan 27, 2019

jpomfret left a comment

jpomfret commented Jan 27, 2019

jpomfret left a comment

jpomfret left a comment

johlju left a comment

jpomfret left a comment

jpomfret left a comment

johlju left a comment

johlju left a comment

johlju commented Jan 29, 2019

jpomfret commented Jan 29, 2019

johlju commented Jan 29, 2019 •

edited

Loading

MSFT_SqlAgentOperator: Added resource for SQL Server Agent Operators #1255

MSFT_SqlAgentOperator: Added resource for SQL Server Agent Operators #1255

Conversation

jpomfret commented Nov 21, 2018 • edited by joeyaiello Loading

Pull Request (PR) description

Task list

johlju commented Dec 4, 2018

jpomfret commented Dec 14, 2018

codecov-io commented Dec 21, 2018 • edited Loading

Codecov Report

johlju commented Jan 4, 2019

johlju left a comment

Choose a reason for hiding this comment

jpomfret left a comment

Choose a reason for hiding this comment

jpomfret left a comment

Choose a reason for hiding this comment

jpomfret left a comment

Choose a reason for hiding this comment

jpomfret left a comment

Choose a reason for hiding this comment

stale bot commented Jan 22, 2019

jpomfret commented Jan 22, 2019

mdaniou commented Jan 22, 2019

mdaniou commented Jan 23, 2019

mdaniou left a comment

Choose a reason for hiding this comment

jpomfret left a comment

Choose a reason for hiding this comment

johlju commented Jan 27, 2019

johlju commented Jan 27, 2019 • edited Loading

jpomfret commented Jan 27, 2019

johlju commented Jan 27, 2019 • edited Loading

johlju left a comment

Choose a reason for hiding this comment

johlju left a comment

Choose a reason for hiding this comment

johlju commented Jan 27, 2019

jpomfret left a comment

Choose a reason for hiding this comment

jpomfret commented Jan 27, 2019

jpomfret left a comment

Choose a reason for hiding this comment

jpomfret left a comment

Choose a reason for hiding this comment

johlju left a comment

Choose a reason for hiding this comment

jpomfret left a comment

Choose a reason for hiding this comment

jpomfret left a comment

Choose a reason for hiding this comment

johlju left a comment

Choose a reason for hiding this comment

johlju left a comment

Choose a reason for hiding this comment

johlju commented Jan 29, 2019

jpomfret commented Jan 29, 2019

johlju commented Jan 29, 2019 • edited Loading

jpomfret commented Nov 21, 2018 •

edited by joeyaiello

Loading

codecov-io commented Dec 21, 2018 •

edited

Loading

johlju commented Jan 27, 2019 •

edited

Loading

johlju commented Jan 27, 2019 •

edited

Loading

johlju commented Jan 29, 2019 •

edited

Loading