-
-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement fact memoization #122
Conversation
297231e
to
2a55a70
Compare
2a55a70
to
1d97c71
Compare
Subsequent calls to on_supported_os with the same options should result in the same facts. This can greatly speed up testing by reducing calls to FacterDB.
1d97c71
to
43bb17e
Compare
I missed some cache keys, but I think I have them all now. Locally the tests were green so I think this is ready for review. |
The build failed on out of date components. Doesn't look related to my PR, just something that changed externally and probably also shows up in master. |
@mcanevet could you have a look? |
added the updated agent components |
@DavidS is it OK for you to merge this? |
I'm running some local tests to confirm it working and being beneficial before merging. I've also noticed that https://github.com/puppetlabs/puppet-module-gems/blob/9c488b456b3d3c1bc6fd02718ee95c102eb78211/config/dependencies.yml#L67 currently is excluding RPF v2 from being installed, so I'll fix that too. |
Results from my local tests:
As you can see the file load time was reduced from 3 minutes 20 seconds down to 14 seconds. I consider that a great win (even when it doesn't make a big difference for this particular test suite). |
Verified this version working with puppetlabs-apache unit tests. See voxpupuli/rspec-puppet-facts#122
Verified this version working with puppetlabs-apache unit tests. See voxpupuli/rspec-puppet-facts#122
Thanks for merging this. Looking forward to a release that includes this.
It also helps greatly if you run something like |
Subsequent calls to on_supported_os with the same options should result in the same facts. This can greatly speed up testing by reducing calls to FacterDB.
This currently fails, probably because it's not taking environment variables into account. I did want to submit this since it's at least a start to collaborate.
Fixes #114