some minor enhancements #1
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of relying on the implicit global rule, make it possible to specify the required version. Also, by default use "installed". Also remove a trailing space. Signed-off-by: Gabriel Filion <[email protected]>
Add two sources that make it possible to use a different module to customize the contents of the config file. The first source gives a per-fqdn file and the second gives a general file. Signed-off-by: Gabriel Filion <[email protected]>
The `apt-get update` call now happens after installing `apt-cacher-ng`, thus pulling ~30 files into the cache. I chose not to perform an `apt-get dist-upgrade` out of consideration for those using `make vm` only to test the module.
You can still `make test` to test both `smoke.pp` and `vagrant.pp`.
Also, expanded `Acquire::http::ProxyAutoDetect` text and moved it to earlier in the README.
I used a class variable rather than global for style reasons.
Conflicts: manifests/init.pp
The old way, without "modules/" is deprecated. Signed-off-by: Gabriel Filion <[email protected]>
Signed-off-by: Gabriel Filion <[email protected]>
…tlabs/stdlib as a dependancy
Added configurable proxy detection timeout
Signed-off-by: Gabriel Filion <[email protected]>
dashes in module names are deprecated and should now be avoided. Signed-off-by: Gabriel Filion <[email protected]>
Signed-off-by: Gabriel Filion <[email protected]>
This makes the client::config class a bit more readable. Signed-off-by: Gabriel Filion <[email protected]>
Also chnage group from "root" to 0 for better portability. Signed-off-by: Gabriel Filion <[email protected]>
Using two parameters with almost the same name is really confusing. With the proper value-checking in place, we can reuse $servers for both configuration modes. Bonus: repair condition for if clause in client::config. I had it switched around when removing the "case" blocks. Bonus 2: Give localhost as the default server. Specifying an empty list of servers as a default value was not a good idea. With the new default value, the worst case will be that a server will wrongly try to connect to itself, but the module will configure things coherently still. Signed-off-by: Gabriel Filion <[email protected]>
client::config is easier to figure out this way, and we can reuse resources to remove unwanted parts. we're actually doing a better cleanup now. Signed-off-by: Gabriel Filion <[email protected]>
usually apt-cacher-ng are on port 3142 so it should make sense to provide a default that uses this port. Signed-off-by: Gabriel Filion <[email protected]>
Signed-off-by: Gabriel Filion <[email protected]>
Signed-off-by: Gabriel Filion <[email protected]>
The block is too much indented and so it is interpreted verbatim Signed-off-by: Gabriel Filion <[email protected]>
Sorry I mixed up what you had contributed Signed-off-by: Gabriel Filion <[email protected]>
Some admin functionality in the web interface need to have admin auth setup. Signed-off-by: Gabriel Filion <[email protected]>
Signed-off-by: Gabriel Filion <[email protected]>
Signed-off-by: Gabriel Filion <[email protected]>
If one tries to use the apt_cacher_ng module to setup a server and client on the same machine, the following error can be seen: Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Duplicate declaration: Anchor[begin] is already declared in file /etc/puppet/environments/production/modules/apt_cacher_ng/manifests/client.pp:21; cannot redeclare at /etc/puppet/environments/production/modules/apt_cacher_ng/manifests/init.pp:18 on node debianpackages-vm-inx01.intra.local.ch This is also problematic with other modules that might declare anchors with the same name, so the solution is to use the module and class name to create a namespace for the anchors.
Reviewed all changes and they all make sense. Timeout value was already present as a configurable variable so we'll keep this our way (but thanks to lluis for that change initially!)
previous authors were consulted in an issue on github: #3
apparently I've been using an antipattern in puppetlabs's style guide. placing the arrows at the beginning of the line makes it more obvious that there is an ordering going on. otherwise you need to spot the arrow at the end of the line.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first commit gives the possibility to specify the package version that is required to be installed. Also, it makes the default value of the 'ensure' to "installed" so that it doesn't rely on the global setting.
The second commit lets us use a different module to customize config files. This way, this module stays general-purpose.