xWebsite - Implemented support for non-HTTP bindings #69

SNikalaichyk · 2015-12-11T16:51:18Z

Implemented support for the following binding protocols: "msmq.formatname", "net.msmq", "net.pipe", "net.tcp".
Implemented support for setting the "EnabledProtocols" property.
Test-* functions updated to return $true if the tested object is valid, is in the desired state, and does not require modification.
The module was redesigned and refactored.
Unit tests updated.

- Implemented support for the following binding protocols: "msmq.formatname", "net.msmq", "net.pipe", "net.tcp". - Implemented support for setting the "EnabledProtocols" property. - Test-* functions updated to return $true if the tested object is valid, is in the desired state, and does not require modification. - The module was redesigned and refactored. - Unit tests updated.

msftclas · 2015-12-11T16:51:21Z

Hi @SNikalaichyk, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

tysonjhayes · 2015-12-15T23:05:51Z

DSCResources/MSFT_xWebsite/MSFT_xWebsite.psm1

@@ -110,6 +118,7 @@ function Set-TargetResource
        [String]
        $State = 'Started',

+        [ValidateLength(1, 64)]


Could you help me understand this change and what we're hoping to acomplish?

The application pool name must contain between 1 and 64 characters.

Further refactoring

TravisEz13 · 2016-01-13T22:55:03Z

DSCResources/MSFT_xWebsite/MSFT_xWebsite.schema.mof

+    [Required] String PhysicalPath;
+    [Write,ValueMap{"Started","Stopped"},Values{"Started", "Stopped"}] String State;
+    [Write] String ApplicationPool;
+    [Write, EmbeddedInstance("MSFT_xWebBindingInformation"), Description("Hashtable containing binding information")] String BindingInfo[];


This isn't a hash table, but instead an array of embedded instances. The description should be improved.

TravisEz13 · 2016-01-13T23:46:59Z

Add-windowsfeature should not be needed for unit test, and is not included in the current unit test suite. Please merge with the latest code.

CIM class versions fixed. Test script updated to use the up-to-date template.

MSFT_xWebsite.Tests.ps1 was ignored by mistake. Removed.

SNikalaichyk · 2016-01-15T12:58:14Z

Hi @TravisEz13,
Your requirements have been implemented.

TravisEz13 · 2016-01-15T19:37:13Z

MSFT_xWebsite.psm1 line 866-869

        catch
        {
            throw $_.Exception.Message
        }

You should throw the exception, not the message.

tysonjhayes · 2016-01-15T19:42:41Z

MSFT_xWebsite.psm1 - General
Could we please move any verbose messages into the LocalizedData and call them like:

Write-Verbose -Message ( @( "$($MyInvocation.MyCommand): "
        $($LocalizedData.Key) -f $Parameter
        ) -join '')

When we eventually get around to PRs that do translation of the strings we can reference the right things.

MSFT_xWebsite.psm1 - Line - 51/140
Could we change this to use the Helper Module located in the root of the DSCResource Folder so we consistently throw the same message?

MSFT_xWebsite.psm1 - Line 94
Why are we adding Id as a return in Get-TargetResource? It doesn't look like we've added it as a parameter else where. Could we remove it?

MSFT_xWebsite.psm1 - Line 344
Why is this changed to elseif from else? Ensure can only be 'Absent' or 'Present' so we have no need for a third option, could we please revert this?

MSFT_xWebsite.psm1 - Line 530
Out of curisoty why are we filtering this to http/https can we have duplicate bindings with other protocols?

MSFT_xWebsite.psm1 - Line 946/947
Could you change this to [ValidateNotNullOrEmpty()] that covers both of these checks.

MSFT_xWebsite.psm1 - Line 689
In the Format-IPAddressString you throw an exception if it can't format but then catch and rethrow the exception. Let's just move this catch into the Format-IPAddress and handle it there (or just remove the try/catch in format-ipaddress and bubble it up here).

- Verbose messages moved to LocalizedData. - Updated to use helper module

SNikalaichyk · 2016-01-16T19:21:41Z

All of your requirements have been implemented.

MSFT_xWebsite.psm1 - Line 946/947
Could you change this to [ValidateNotNullOrEmpty()] that covers both of these checks.

I allow null and empty strings to be passed to the Format-IPAddressString and Test-PortNumber helper functions, not disallow.

MSFT_xWebsite.psm1 - Line 530
Out of curisoty why are we filtering this to http/https can we have duplicate bindings with other protocols?

Yes, it is possible to add identical non-standard bindings (such as "net.tcp") to different sites.

TravisEz13 · 2016-01-26T21:40:28Z

Please include a summary of changes in an unreleased version change per the contribution guidelines

* Test scripts updated to use the latest templates. * README updated.

SNikalaichyk · 2016-01-27T14:10:20Z

Can you please re-run AppVeyor build? For some reason it timed out during the last commit.

TravisEz13 · 2016-01-27T19:57:45Z

I triggered a rerun in AppVeyor. It hasn't started yet.

tysonjhayes · 2016-01-27T22:33:30Z

LGTM. Thanks @SNikalaichyk

xWebsite - Implemented support for non-HTTP bindings

SNikalaichyk added 4 commits December 4, 2015 13:38

Merge remote-tracking branch 'refs/remotes/PowerShell/dev' into dev

a0e1822

Merge remote-tracking branch 'refs/remotes/PowerShell/dev' into dev

b05b322

Merge remote-tracking branch 'refs/remotes/PowerShell/dev' into dev

83dd8f8

msftclas added the cla-already-signed label Dec 11, 2015

SNikalaichyk changed the title ~~Implemented support for non-HTTP bindings~~ xWebsite - Implemented support for non-HTTP bindings Dec 11, 2015

tysonjhayes reviewed Dec 15, 2015
View reviewed changes

SNikalaichyk added 2 commits December 16, 2015 20:10

Support for non-HTTP bindings

cb424e8

Implemented support for non-HTTP bindings

13876d4

Further refactoring

KarolKaczmarek added the needs review The pull request needs a code review. label Jan 6, 2016

TravisEz13 reviewed Jan 13, 2016
View reviewed changes

TravisEz13 added waiting for author response The pull request is waiting for the author to respond to comments in the pull request. and removed needs review The pull request needs a code review. labels Jan 14, 2016

SNikalaichyk added 4 commits January 14, 2016 14:41

Merge remote-tracking branch 'refs/remotes/PowerShell/dev' into dev

4ce6590

Implemented support for non-HTTP bindings

6f99bf8

CIM class versions fixed. Test script updated to use the up-to-date template.

Merge remote-tracking branch 'refs/remotes/PowerShell/dev' into dev

de85ec2

gitignore fix

6cae860

MSFT_xWebsite.Tests.ps1 was ignored by mistake. Removed.

SNikalaichyk mentioned this pull request Jan 15, 2016

MSFT_xWebsite support for FTP websites #81

Open

Updated to use helper module

d708967

- Verbose messages moved to LocalizedData. - Updated to use helper module

TravisEz13 added needs review The pull request needs a code review. and removed waiting for author response The pull request is waiting for the author to respond to comments in the pull request. labels Jan 21, 2016

Implemented support for non-HTTP bindings

f6688a1

* Test scripts updated to use the latest templates. * README updated.

tysonjhayes pushed a commit that referenced this pull request Jan 27, 2016

Merge pull request #69 from SNikalaichyk/dev

39d1ab7

xWebsite - Implemented support for non-HTTP bindings

tysonjhayes merged commit 39d1ab7 into dsccommunity:dev Jan 27, 2016

vors removed the needs review The pull request needs a code review. label Jan 27, 2016

rs38 mentioned this pull request Jun 7, 2016

feature request: xWebApplication.EnabledProtocols #145

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xWebsite - Implemented support for non-HTTP bindings #69

xWebsite - Implemented support for non-HTTP bindings #69

SNikalaichyk commented Dec 11, 2015

msftclas commented Dec 11, 2015

tysonjhayes Dec 15, 2015

SNikalaichyk Dec 16, 2015

TravisEz13 Jan 13, 2016

TravisEz13 commented Jan 13, 2016

SNikalaichyk commented Jan 15, 2016

TravisEz13 commented Jan 15, 2016

tysonjhayes commented Jan 15, 2016

SNikalaichyk commented Jan 16, 2016

TravisEz13 commented Jan 26, 2016

SNikalaichyk commented Jan 27, 2016

TravisEz13 commented Jan 27, 2016

tysonjhayes commented Jan 27, 2016

xWebsite - Implemented support for non-HTTP bindings #69

xWebsite - Implemented support for non-HTTP bindings #69

Conversation

SNikalaichyk commented Dec 11, 2015

msftclas commented Dec 11, 2015

tysonjhayes Dec 15, 2015

Choose a reason for hiding this comment

SNikalaichyk Dec 16, 2015

Choose a reason for hiding this comment

TravisEz13 Jan 13, 2016

Choose a reason for hiding this comment

TravisEz13 commented Jan 13, 2016

SNikalaichyk commented Jan 15, 2016

TravisEz13 commented Jan 15, 2016

tysonjhayes commented Jan 15, 2016

SNikalaichyk commented Jan 16, 2016

TravisEz13 commented Jan 26, 2016

SNikalaichyk commented Jan 27, 2016

TravisEz13 commented Jan 27, 2016

tysonjhayes commented Jan 27, 2016