John

Cheers for the response. 

I guess I could have explained the issue a bit better :) 
By pattern, I'm referring to using a params.pp class with variable 
interpolation... I.e.

# --glassfish/manifests/params.pp#25

 $glassfish_download_mirror = "http://download.java.net/glassfish/
> ${glassfish::version}/release"


This var is then used as the default for:

>  # --glassfish/manifests/init.pp#102

 $download_mirror = $glassfish::params::glassfish_download_mirror,


'*$glassfish::version*' is defaulted in *glassfish/manifests/params.pp#37 *to 
'3.1.2.2', and exposed as a class param in *glassfish/manifests/init.pp#121, 
*allowing users to plug-in other versions*. *

The download URL is then put together in *glassfish/manifests/install.pp#79*. 
However the issue with the failing tests is that *${glassfish::version}* above 
is evaluating to an empty string, resulting in a download path of "
http://download.java.net/glassfish//release/glassfish-3.1.2.2.zip";. 
In-between the double '/' should be the *$version* value.

What I was aiming for is to have a default mirror that looks at java.net 
and uses the passed in *${glassfish::version} *value, but allow people to 
plug in an internal mirror URL, which likely won't have any variables in 
it. 
So when running with defaults, the above URL should be as per the test 
expectation...  

Cheers
Gav

On Monday, 12 January 2015 15:04:39 UTC, jcbollinger wrote:
>
>
>
> On Monday, January 12, 2015 at 5:00:04 AM UTC-6, Gavin Williams wrote:
>>
>> Morning all
>>
>> I'm trying to add some functionality to my Puppet-Glassfish[1] module to 
>> support providing an alternative download mirror.
>>
>> I've made the initial code changes in caa445631d[2], however a couple of 
>> my tests are now failing[3]. 
>>
>> I believe the issue is due to using the '$glassfish::version' variable 
>> within my params.pp[4] file which is also exposed as class variables[5] 
>> which can then be over-ridden on calling. 
>> I've tried using both the fully qualified '$glassfish::version' variable, 
>> aswell as just the local '$version' variable, but neither seem to work. 
>>
>> The tests are complaining that the generated 
>> '$glassfish::glassfish_download_mirror' variable doesn't include a version 
>> number:
>>
>>> expected that the catalogue would contain 
>>> Exec[download_glassfish-3.1.2.2.zip] with command set to "wget -q 
>>> http://download.java.net/glassfish/3.1.2.2/release/glassfish-3.1.2.2.zip 
>>> -O /tmp/glassfish-3.1.2.2.zip" but it is set to "wget -q 
>>> http://download.java.net/glassfish//release/glassfish-3.1.2.2.zip -O 
>>> /tmp/glassfish-3.1.2.2.zip"
>>>
>> Note the double '//' between *glassfish *and *release*.
>>
>> Should this pattern work? 
>> Or how should I be doing it? 
>>
>>
>
> I'm not sure what you mean by "this pattern", but it is certainly 
> system-dependent whether the URL you are now producing refers to the same 
> underlying physical resource as the one your tests expect, or indeed, 
> whether it can possibly refers to any physical resource at all.  You should 
> certainly fix the problem highlighted by the tests (and kudos to you for 
> having tests that picked this up!).
>
> In my experience, a double slash in a path normally arises one of two ways:
>
>    1. from concatenating a path ending in a slash with one beginning with 
>    a slash, or
>    2. from joining a two strings with a '/' delimiter, when the first one 
>    already ends with a slash or the second begins with one.
>
> Supposing that one of those is what's happening here, the solution is 
> probably to be very clear about what each parameter and variable 
> represents, to provide only valid values for them, and to perform on them 
> only operations that are correct for those entities.
>
> Specifically, if you have a variable containing the value 
> "/release/glassfish-3.1.2.2.zip", then what, exactly, is that variable 
> supposed to represent?  It is not a relative path, but apparently it's not 
> a correct absolute path, either.  I can't think of what a variable might 
> represent in your module for such a value to be valid for it, so if one of 
> your variables has such a value then that is probably incorrect.  
> Alternatively, if you have a variable expected to contain a valid directory 
> URL (which must end in a '/'), and in fact containing the valid directory 
> URL "http::/java.download.net/glassfish/", and you join the value of that 
> variable to a relative path, then you should not put a '/' between the 
> parts because you can rely on the base URL already containing one.
>
> To the extent that you may not consider data fed to your module from 
> outside to be completely reliable, you should consider implementing 
> validation checks.
>
>
> John
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/puppet-users/fd265aff-7bac-497b-a20a-07a6301fb045%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to