Mirror url custom port #121
Merged
Conversation
This file contains 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
added 15 commits
October 19, 2016 17:13
…ror-url-custom-port
bastelfreak
reviewed
Dec 26, 2016
| @@ -11,14 +12,18 @@ | |||
| service_provider: 'upstart' | |||
| } | |||
| end | |||
| let :params do | |||
| let :common_params do | |||
|
thanks! |
elmendalerenda
pushed a commit
to elmendalerenda/puppet-kafka
that referenced
this pull request
Mar 30, 2018
* support custom port in mirror_url * refactor mirror_url validation to a shared example * make mirror_url consistent in broker class and add shared example to test this * make mirror_url consistent in consumer class and add shared example to test this * make mirror_url consistent in producer class and add shared example to test this * factor out repeated mirror_url regex to $kafka::params::mirror_url_regex * support custom port in mirror_url * refactor mirror_url validation to a shared example * make mirror_url consistent in broker class and add shared example to test this * make mirror_url consistent in consumer class and add shared example to test this * make mirror_url consistent in producer class and add shared example to test this * factor out repeated mirror_url regex to $kafka::params::mirror_url_regex * ruuubooocooop! <waves fist> * ruuubooocooop! <waves fist>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This change makes validation of mirror_url consistent across broker/producer/consumer/mirror classes, allowing any of these to include a custom port#
It also fixes an error in the optional /path/to/package part of the original URL regex, which effectively caused the *.tld 2-6 character length restriction to be ignored.
Tests for the various elements of the URL regex are run combinatorially – ie. every possible combination is tested. Since these are repeated for each of the 4 dependent classes, running all unit tests takes ~5mins. Testing a few specific sample URLs would be quicker, but risks missing some edge cases. Really this highlights that this common validation should be extracted into a custom puppet function, unit tested once there, and mocked out in tests of dependent classes. That’s beyond the scope of this change though.