FOR REVIEW: Port PuppetDB to use trapperkeeper#773
FOR REVIEW: Port PuppetDB to use trapperkeeper#773nwolfe wants to merge 6 commits intopuppetlabs:masterfrom nwolfe:feature/master/pe2015-refactor-to-trapperkeeper
Conversation
This commit does the following: * Introduces a `context` atom in services.clj, whose value is a map used to keep references to the various stateful bits of the PuppetDB subsystems. This is something that has been discussed previously as a means for potentially allowing more interactive development in the REPL, and for doing things like dynamically reloading subsystems when configuration changes occur. In this commit, it's used to provide a way to shutdown those subsystems when a fatal error occurs. * Adds the new `error-shutdown!` function, which is responsible for shutting down threads and subsystems from the `context` when a fatal error has occurred.
By default, leinengen doesn't allow SNAPSHOT dependencies when running `uberjar` on a project that is not also a SNAPSHOT. Because of the way that we handle the project version in the PuppetDB project.clj file, PuppetDB is *never* in a SNAPSHOT state. This commit adds an environment variable that will cause leinengen to allow SNAPSHOT deps, but only when running acceptance tests FROM SOURCE. Packaging jobs should still fail if this kind of scenario is encountered for now.
|
This commit should fix one bucket o' failing tests. Committing it so we can get a test run - another commit to fix the rest of the tests coming soon. |
|
🔴 Test failed. |
|
CLA signed by all contributors. |
|
retest this please |
1 similar comment
|
retest this please |
|
🔴 Test failed. |
|
🔴 Test failed. |
|
retest this please |
|
🔴 Test failed. |
|
💚 Test passed. |
|
🔴 Test failed. |
|
retest this please |
|
🔴 Test failed. |
|
retest this please |
|
💚 Test passed. |
|
@grimradical @senior @kbarber whoo-hoo! We had a few squirrely command-line / test issues to sort out, but everything seems happy and green now. At this point we're ready to do a release of trapperkeeper and then tweak this PR to point at the non-SNAPSHOT version, but are planning to wait a bit to hear whether or not you guys have any feedback on the PR before we do that. Let us know if you have any thoughts! |
|
💚 Test passed. |
There was a problem hiding this comment.
@cprice404 I think we talked about this. Does this go away once we have a release of TK and KS?
There was a problem hiding this comment.
It can, certainly. Not sure if it's harmful, though, as this is only used in tests that are being run from source (e.g. when acceptance runs against pull requests). It might be handy in the future, but I've got no problem removing it if you guys prefer.
There was a problem hiding this comment.
Can a comment be added here with a pointer to some docs on what this file does?
There was a problem hiding this comment.
heh. This file doesn't actually support comments yet (although we already have a jira in flight in the current sprint to address this).
We should be able to get that change in before we do a release, so we should be able to update this PR to include that.
Meantime, if you're interested: https://github.com/puppetlabs/trapperkeeper#bootstrapping
|
Thanks, @senior . So, here's my laundry list:
Does that sound accurate? |
|
This was closed in favor of #780 , which is rebased and addresses all of the comments on this PR. |
5 participants
This PR is entirely ready to go as far as we are aware, with one minor exception: we'd like to do an actual release of trapperkeeper and change the dependency here away from being a SNAPSHOT. I'm holding off on that until I get some feedback from you guys, in case there are any final changes we need to sneak into TK on your behalf. But we're ready to do a release unless you guys find something awry in here.
We did grab your latest Jetty SSL stuff and patch it in to TK, so I'm pretty sure everything's in sync.