concourse / pool-resource

atomically manages the state of the world (e.g. external environments)
Apache License 2.0
54 stars 36 forks source link

Correct examples so they release locks on failure #23

Closed jfuerth closed 7 years ago

jfuerth commented 7 years ago

The examples as written would not give back the locks if the build fails or aborts. Updated to use ensure.

pivotal-issuemaster commented 7 years ago

@jfuerth Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

concourse-bot commented 7 years ago

Hi there!

We use Pivotal Tracker to provide visibility into what our team is working on. A story for this issue has been automatically created.

The current status is as follows:

This comment, as well as the labels on the issue, will be automatically updated as the status in Tracker changes.

pivotal-issuemaster commented 7 years ago

@jfuerth Thank you for signing the Contributor License Agreement!

pivotal-issuemaster commented 7 years ago

@jfuerth Thank you for signing the Contributor License Agreement!

chendrix commented 7 years ago

Hi there!

I am hesitant to merge this PR. The examples given in the README are the sort of "minimal" use case. While using an ensure in your puts to always release the locks is a valuable pattern, it may not actually be the intended use case.

For instance, if you are running long-running tests against an AWS environment, and it fails, and you need to go in and investigate logs on VMs in AWS, you want the lock to not have been released when your tests failed. If they had, another build may have picked up the environment lock and deployed/tested again without giving you the chance to investigate the failure.

I do want to capture your use case, though, so I'm labeling the tracker story with docs label. This will allow us to keep track of it even though I'm closing this PR.