Rename vhost to server.d #348

renich · 2014-06-22T22:17:20Z

I'd rename vhost to server.d for the following reasons:

vhost is an apache term: Virtual Hosts. NginX's term is Server.
The .d convention is used widely accross services and distributions.

jfryman · 2014-06-24T14:32:17Z

PRs gladly accepted!

wyardley · 2016-10-07T16:57:47Z

The .d convention is for files, no?
Personally, I think vhost / virtualhost is a reasonable and generally understood term, but in any event, seems like this is being handled separately, if at all.

Additionally, changing it would be backwards incompatible and for very little benefit.

I propose we close this one without further action; it's now > 2 years later, and the module is still using vhost (and I would even say that IMHO, that's not a bad thing).

renich · 2016-10-07T18:02:53Z

Well, it's more about convention and standards. Also, being accurate in the terms used.

I invite you to go to the #nginx IRC channel @ Freenode and say something about vHosts. They will say: "we are not apache.".

Besides, that's broken logic. You might start calling '/etc' '/configuration' or mounting stuff in '/mystuff'. That doesn't make it convenient nor good at all.

As obvious as it is, do as you please. It's your repo. Still, I disapprove of the lack of interest in these matters.

wyardley · 2016-10-07T18:15:34Z

I see foo.d used for directories including things, I'm not familiar with using, say, server.d (though the PR you reference uses 'server, rather than server.d, so that seems fine to me).

server is also used to refer to a daemon (such as nginx), or even a system on which such a daemon runs. So from that standpoint, "virtualhost" is a lot clearer to me than "server" in terms of nomenclature, even though there are probably some cases where you may be doing things in a server block that are not a virtualhost in the traditional sense. And, I can be as pedantic as the next guy, maybe more so, but you're talking about renaming huge components of the module and introducing changes that will force people to change how they use the module for a change that, IMHO, makes things more confusing / ambiguous, rather than less.

I don't think "virtual host" is only an Apache term, it's a generally and commonly used term that's used in a lot of places, including in RFCs and for things other than http content hosting. Nginx may claim that it's an "Apache term", but regardless, it's common usage. I certainly don't have any interest in getting into an argument with the Nginx folks about their terminology on IRC. https://en.wikipedia.org/wiki/Virtual_hosting

It's not "my repo", I'm just helping out trying to clear out some old issues.

I will defer to the modules original maintainers as well as the Voxpupuli folks. It doesn't seem like the "other PR" that @jfryman referred to in #358 ever got merged, but if you'd like to reopen it and rebase against current master, and / or open a new PR that's based on current master, we can discuss this further. cc: @bastelfreak @3flex

renich · 2016-10-07T18:23:28Z

https://www.nginx.com/resources/wiki/start/topics/examples/server_blocks/

renich · 2016-10-07T18:26:14Z

Besides, in Fedora (and it's children; RHEL, CentOS, Scientific Linux, etc) you have:

# find /etc -type d -iname '*.d'
/etc/profile.d
/etc/X11/fontpath.d
/etc/X11/xorg.conf.d
/etc/X11/xinit/xinitrc.d
/etc/bash_completion.d
/etc/pm/config.d
/etc/pm/power.d
/etc/pm/sleep.d
/etc/xinetd.d
/etc/yum.repos.d
/etc/libreport/events.d
/etc/libreport/workflows.d
/etc/ld.so.conf.d
/etc/popt.d
/etc/chkconfig.d
/etc/rc.d
/etc/rc.d/init.d
/etc/rc.d/rc0.d
/etc/rc.d/rc1.d
/etc/rc.d/rc2.d
/etc/rc.d/rc3.d
/etc/rc.d/rc4.d
/etc/rc.d/rc5.d
/etc/rc.d/rc6.d
/etc/request-key.d
/etc/depmod.d
/etc/modprobe.d
/etc/security/console.perms.d
/etc/security/limits.d
/etc/security/namespace.d
/etc/security/pwquality.conf.d
/etc/pam.d
/etc/sysctl.d
/etc/logrotate.d
/etc/gss/mech.d
/etc/prelink.conf.d
/etc/dbus-1/system.d
/etc/rsyslog.d
/etc/yum/protected.d
/etc/yum/pluginconf.d
/etc/yum/fssnap.d
/etc/binfmt.d
/etc/modules-load.d
/etc/systemd/system/php-fpm.service.d
/etc/systemd/system/sssd.service.d
/etc/systemd/system/redis-sentinel.service.d
/etc/systemd/system/redis.service.d
/etc/tmpfiles.d
/etc/udev/rules.d
/etc/udev/hwdb.d
/etc/polkit-1/rules.d
/etc/polkit-1/localauthority/10-vendor.d
/etc/polkit-1/localauthority/20-org.d
/etc/polkit-1/localauthority/30-site.d
/etc/polkit-1/localauthority/50-local.d
/etc/polkit-1/localauthority/90-mandatory.d
/etc/polkit-1/localauthority.conf.d
/etc/reader.conf.d
/etc/cron.d
/etc/grub.d
/etc/dnsmasq.d
/etc/dracut.conf.d
/etc/NetworkManager/dispatcher.d
/etc/NetworkManager/dispatcher.d/pre-down.d
/etc/NetworkManager/dispatcher.d/pre-up.d
/etc/NetworkManager/dispatcher.d/no-wait.d
/etc/NetworkManager/dnsmasq.d
/etc/NetworkManager/conf.d
/etc/NetworkManager/dnsmasq-shared.d
/etc/rwtab.d
/etc/statetab.d
/etc/dhcp/dhclient.d
/etc/sssd/conf.d
/etc/smartmontools/smartd_warning.d
/etc/kernel/postinst.d
/etc/audisp/plugins.d
/etc/audit/rules.d
/etc/setuptool.d
/etc/sudoers.d
/etc/php-zts.d
/etc/php.d
/etc/fonts/conf.d
/etc/nginx/conf.d
/etc/nginx/include.d
/etc/nginx/default.d
/etc/nginx/server.d
/etc/php-fpm.d
/etc/my.cnf.d
/etc/mercurial/hgrc.d
/etc/oddjobd.conf.d
/etc/dovecot/conf.d
/etc/dnf/protected.d
/etc/gdbinit.d
/etc/httpd/conf.d
/etc/libibverbs.d
/etc/exports.d
/etc/salt/minion.d
/etc/salt/cloud.conf.d
/etc/salt/cloud.deploy.d
/etc/salt/cloud.maps.d
/etc/salt/cloud.profiles.d
/etc/salt/cloud.providers.d
/etc/salt/master.d
/etc/cockpit/ws-certs.d
/etc/auto.master.d
/etc/dpkg/dpkg.cfg.d
/etc/krb5.conf.d
/etc/storaged/modules.conf.d
/etc/libblockdev/conf.d
/etc/fail2ban/action.d
/etc/fail2ban/fail2ban.d
/etc/fail2ban/filter.d
/etc/fail2ban/jail.d

... ¬_¬

Also, on Debian and children, you have these examples as well. Gentoo/Funtoo haver them. Arch Linux has them. The list goes on and on. directory.d serves as a container for included configuration; like it or not. It is a convention most distros use.

wyardley · 2016-10-07T18:32:20Z

Yes, that is also the convention that's used by this module (e.g., $conf_dir/conf.d), so I was just initially confused what you're suggesting be changed -- are you taking about the directory names, or the name of the block itself?

The whole sites-enabled / available thing is a Debian / Ubuntu thing... I don't personally like it or use it (and have #878 open to just put the vhosts in conf.d directly), but I think renaming them to vhosts.d or similar would break whatever tooling that system has to manipulate those symlinks.

It sounds like @bastelfreak is amenable to doing the rename, though it's really going to create a lot of work for people with existing PRs based against the current codebase.

bastelfreak · 2016-10-07T18:43:52Z

I think we should stick with the preferred name from the nginx devs. A rename is a lot of work and a huge breaking change. We have to announce that in a minor release and do it in a major one.

renich · 2016-10-07T18:45:06Z

So be it. vhost is not the prefered term or name of the nginx devs.

wyardley · 2016-10-07T20:35:27Z

@renich: right, @bastelfreak is agreeing with you.

So, as I see it, there are a few points of consideration:

The old (closed) substituted the term vhost for server #358 is probably not safe to apply or even rebase; @renich: are you willing to create a new PR renaming 'server' to 'vhost'? For the purposes of that PR, I think we should leave out discussion of the naming of the directories where the configs are stored (none of which have vhost in them, as best I remember).
There had been some discussion about moving vhost / location / etc. out of the resource namespace; should that be tied into this major release (since it would also be a change that may break stuff if people are invoking nginx::resource::foo directly), and is anyone willing to undertake that work?
Lastly, I know for Puppetlabs modules, they'll usually include a deprecation warning when something is renamed like this (so that if someone references nginx::vhost, they'll get an explicit failure message. Is that something Voxpupuli generally encourages / requires, and if so, can that be added into the PR? That way, people will get an error message rather than just a failed compilation.

renich · 2016-10-17T22:00:08Z

The old (closed) substituted the term vhost for server #358 is probably not safe to apply or even rebase; @renich: are you willing to create a new PR renaming 'server' to 'vhost'? For the purposes of that PR, I think we should leave out discussion of the naming of the directories where the configs are stored (none of which have vhost in them, as best I remember).

Yeah, I can do this. It's been a long time since I used puppet but I don't think it is that hard. I will take a look at it and re-offer it.

There had been some discussion about moving vhost / location / etc. out of the resource namespace; should that be tied into this major release (since it would also be a change that may break stuff if people are invoking nginx::resource::foo directly), and is anyone willing to undertake that work?

I don't think I have the time for this. Sorry.

Lastly, I know for Puppetlabs modules, they'll usually include a deprecation warning when something is renamed like this (so that if someone references nginx::vhost, they'll get an explicit failure message. Is that something Voxpupuli generally encourages / requires, and if so, can that be added into the PR? That way, people will get an error message rather than just a failed compilation.

Well, I can take a look at how to do it and, possibly, do it. I'll let you know.

Based on the discussion we had, I've renamed all instances of 'vhost' to 'server'. The main reason for this is that NginX does not uphold the concept of vHost: https://www.nginx.com/resources/wiki/start/topics/examples/server_blocks/ Stating: "Note: “VirtualHost” is an Apache term. NGINX does not have Virtual hosts, it has “Server Blocks” that use the server_name and listen directives to bind to tcp sockets." Besides, the extended use of the concept causes confusion and misunderstanding when it comes to NginX. I still need to work on two more requests.

renich · 2016-10-17T22:21:28Z

OK, about the deprecation warning, what would be the suggestion here? Should use aliases and try to write down some logic that triggers a message when the alias is used?

stealthybox · 2016-11-01T01:23:37Z

I support this.
Configuring vhosts in puppet is hard to think about since the nginx configs don't have them.
servers and locations are the proper terminology to use when managing nginx.

This is a large change for the module though!

The alias option is good.
Making nginx::server and nginx::vhost equivalent would let the module be backwards compatible.

If a hard shift is necessary, a soft change could be implemented with deprecation message for a few minor versions until a major version deprecates the change.

This closes voxpupuli#348 and closes voxpupuli#932. It was created by running git ls-files | xargs -i{} sed -i -e 's/vhost/server/g' {} and manually fixing lint issues and renaming all files called '*vhost*'.

wyardley · 2016-11-10T18:56:15Z

Handling in #968

renich mentioned this issue Sep 12, 2014

substituted the term vhost for server #358

Closed

wyardley closed this as completed Oct 7, 2016

wyardley reopened this Oct 7, 2016

renich mentioned this issue Oct 17, 2016

Vhosts? saltstack-formulas/nginx-formula#105

Closed

DavidS mentioned this issue Nov 10, 2016

Rename all vhost ocurrences to server #968

Closed

wyardley closed this as completed Nov 10, 2016

axolander mentioned this issue Jan 18, 2017

incompatible with puppet/nginx v0.6.0 pgassmann/puppet-letsencrypt_nginx#9

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename vhost to server.d #348

Rename vhost to server.d #348

renich commented Jun 22, 2014

jfryman commented Jun 24, 2014

wyardley commented Oct 7, 2016

renich commented Oct 7, 2016 •

edited

Loading

wyardley commented Oct 7, 2016 •

edited

Loading

renich commented Oct 7, 2016

renich commented Oct 7, 2016

wyardley commented Oct 7, 2016

bastelfreak commented Oct 7, 2016

renich commented Oct 7, 2016 •

edited

Loading

wyardley commented Oct 7, 2016

renich commented Oct 17, 2016

renich commented Oct 17, 2016

stealthybox commented Nov 1, 2016

wyardley commented Nov 10, 2016

Rename vhost to server.d #348

Rename vhost to server.d #348

Comments

renich commented Jun 22, 2014

jfryman commented Jun 24, 2014

wyardley commented Oct 7, 2016

renich commented Oct 7, 2016 • edited Loading

wyardley commented Oct 7, 2016 • edited Loading

renich commented Oct 7, 2016

renich commented Oct 7, 2016

wyardley commented Oct 7, 2016

bastelfreak commented Oct 7, 2016

renich commented Oct 7, 2016 • edited Loading

wyardley commented Oct 7, 2016

renich commented Oct 17, 2016

renich commented Oct 17, 2016

stealthybox commented Nov 1, 2016

wyardley commented Nov 10, 2016

renich commented Oct 7, 2016 •

edited

Loading

wyardley commented Oct 7, 2016 •

edited

Loading

renich commented Oct 7, 2016 •

edited

Loading