Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated to work with Puppet 4 and updated IIS module #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gceraso
Copy link

@gceraso gceraso commented Aug 29, 2016

  • Updated syntax to match IIS module.
  • Cleaned up and minimized variables
  • Removed params class and simplified module
  • Sanitized indents.

@ferventcoder
Copy link
Contributor

This is a good start. I'll review what's included here.

$server_install_location = 'C:\tools\chocolatey.server',
$chocolatey_server_app_pool_name = 'chocolatey.server',
$chocolatey_server_app_port = $port,
) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer the params pattern because it allows for hiera values. It's possible this will also allow that, but I'm unsure. And keep in mind this should also continue to work with v3.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because these are now parameterized, they will still be looked up via hiera.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will that still be compatible with both versions? Having the extra logic here doesn't hurt anything that I'm aware of, so I'm not ready to modernize until v3 EOLs completely.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automatic parameter lookup also works in v3.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well then there really isn't a reason to be doing params pattern then. Unless there is something I'm not thinking of offhand

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. AFAIK Puppet recommends not using params pattern going forward.

@ferventcoder
Copy link
Contributor

app_pool => 'DefaultAppPool'
iis_site {'Default Web Site':
ensure => stopped,
app_pool => 'DefaultAppPool'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a different change than what the module originally did. It removed the default website. Are you proposing to just stop it instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am simply because there is a bug which I meant to include. When no sites exist, this creates an issue. voxpupuli/puppet-iis#123

@gceraso
Copy link
Author

gceraso commented Aug 29, 2016

As for https://github.com/chocolatey/puppet-chocolatey_server/blob/master/metadata.json#L48 I'm trying to locate when the syntax changed. Do you have a preference here, I can point the latest version.

@ferventcoder
Copy link
Contributor

@gceraso I think if we can determine what version of IIS, we can maintain compat with the old and the new.

@gceraso
Copy link
Author

gceraso commented Aug 29, 2016

The best I can gather is it seems the syntax changed for "2015-05-22 - Release 1.4.1"

@ferventcoder
Copy link
Contributor

Wow, syntax change and not a major version bump?!

@ferventcoder
Copy link
Contributor

Unless they left it compatible with the older method of calling iis_site?

@gceraso
Copy link
Author

gceraso commented Aug 29, 2016

Seems like that's when they started their rewrite. The 2.0 release seems to be when they cut support for the old syntax.

@ferventcoder
Copy link
Contributor

Okay, whew... 👍

@gceraso
Copy link
Author

gceraso commented Aug 30, 2016

Updated the metadata file with the relevant versions for IIS.

@GerbenWelter
Copy link

The currently used IIS module is deprecated since August 2017. The module now refers to the supported PuppetLabs IIS module.

@lmayorga1980
Copy link

👍

@lmayorga1980
Copy link

when is this going to be merged? puppet-iis is already deprecated

@witsec
Copy link

witsec commented Jul 19, 2018

Any update here regarding the dependancy on puppet/iis?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants