Issue #2847 has been updated by Ken Barber.

> > * The need for mutex troubles me - I'm curious to understand if this is a 
> > bug in the latest version of facter that this fact will be committed to. If 
> > it is indeed a bug we should get it raised somewhere - and then accept this 
> > workaround.
> 
> I put mutex in every fact I did, not only this particular one. I would 
> recommend not spending too much time on pursuing this, and I will simply 
> remove mutexes from my code. On this note, the following use 
> Thread::exclusive for some reason too: processor.rb and util/memory.rb; I 
> guess it is also not needed?

Yeah I noticed that last night while I was digging around. I've asked the dev 
list. If its a bug its a bug and we should work around it. If you've seen bad 
side-effects in later facter then we should definitely work around it ... but 
perhaps a mention of the bug in question in comments is needed.

The closest bug I can find on this is #4246.

I raised a message on the dev forum about it to see if anyone else has seen it 
for themselves:

http://groups.google.com/group/puppet-dev/browse_frm/thread/ccd4a7d762651aa5

> 
> > * There is a new version of mount.rb in KW's repo: 
> > https://github.com/kwilczynski/facter-facts/blob/master/mounts.rb - when I 
> > do copy this in it throws unexpected invocation using the existing moch 
> > arrangement and needs to be fixed before merging.
> 
> Is this related to tests?

Yes totally. And yes - if Michael is doing the tests for you this is probably 
more aimed at him.

> > * The code breaks in ruby 1.9.2 & 1.9.3 due to the use of each. You can see 
> > this easily enough if you switch to ruby 1.9.2 with rvm.
> 
> I am not using 1.9.x anywhere yet. Perhaps later I will set myself up with 
> up-to-date RVM and test it.

We aim for future ruby support so we try to test on it. I think its a trivial 
fix tbh - just a deprecated use of each.

> > Also in regards to naming of the facts ... 'mounts' is probably okay but 
> > 'devices' doesn't make it obvious they are related and is ambiguous. 
> > Perhaps they should be something like:
> > 
> > * mounts_paths
> > * mounts_devices
> > 
> > Or something like that ... since the idea is that they are arrays related 
> > by order having the prefix be the same makes it obvious. What does everyone 
> > think? Doing this kind of thing without proper structured data can be so 
> > ugly at the best of times :-).
> 
> What about "mount_points" instead of "mounts_paths"?

mount_points and mount_devices is fine ... probably to match the current 
convention the file should be mount.rb then?
----------------------------------------
Feature #2847: mountpoint fact
https://projects.puppetlabs.com/issues/2847

Author: anarcat -
Status: Code Insufficient
Priority: Normal
Assignee: Michael Kincaid
Category: library
Target version: 
Keywords: mounts, devices
Branch: 
Affected Facter version: 1.6.1rc4


I have implemented, based on work by the Debian's Sysadmin team, a fact that 
allows listing of the mountpoints (and associated devices). I have attached the 
code for the fact, which is called "mountpoints" but also exports "devices".

I have requested and got approval from the original author (Stephen Gran, which 
code can still be seen here: 
http://git.debian.org/?p=mirror/dsa-puppet.git;a=blob;f=facts/mounts.rb;hb=HEAD)
 before contacting you. To quote him:

"git blame indicates that all the current code in the mounts fact is mine, so 
I'm happy to license it under the GPL (of any verson, as that is apparently the 
puppet license, at first glance) for purposes of pushing it upstream.  OTOH, it 
is about 20 lines of fairly trivial code, so I'm not sure copyright is even an 
issue."

Apparently, his original code was relying on the presence of the "Filesystem" 
Ruby library, which may not be desirable in this case, which is why I 
implemented a linux workaround. It's my first fact ever, so I'm very curious to 
see if I have done this properly or if there are possible improvements to the 
code. I know comments could be good... 

My working copy of the code is also available through git: 
http://git.koumbit.net/?p=puppet/modules/common.git;a=blob;f=plugins/facter/mountpoints.rb;hb=HEAD

Thanks,

A.


-- 
You have received this notification because you have either subscribed to it, 
or are involved in it.
To change your notification preferences, please click here: 
http://projects.puppetlabs.com/my/account

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Bugs" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-bugs?hl=en.

Reply via email to