search

concourse / pool-resource

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

Revert tolerance of in-place file modifications #19

Closed rainmaker closed 8 years ago

rainmaker commented 8 years ago

Hello,

I'm coming to revert an accepted PR https://github.com/concourse/pool-resource/pull/11 that was made with the goal of allowing in-place modifications of lock files. My team keeps metadata in these files and sometimes needs to change them in batches, which is painful when we can't modify the contents of claimed locks.

After the initial PR was accepted, we realized that it didn't quite work as expected. We knew that the in-place modifications wouldn't be available immediately, but didn't realize how the unclaiming process works. The resource checks out the SHA from when it first claimed the lock and uses that version of the file to unclaim it. The few times we attempted to modify a locked file, the changes were simply reverted when it was unlocked. The intended flow couldn't be fully represented in the integration tests, so we didn't notice until we tried it manually.

We discussed changing the unclaim process to use the latest version of the file, but it seemed like it added complexity to the existing behavior and we still didn't know whether anyone else wanted this feature. In the end, my team decided that we'd prefer to store our metadata separately from the lock files, and therefore don't need this anymore. We'd like to revert our original changes, since the current behavior might be confusing if someone encountered it. Perhaps this work can be used in the future if the full feature is desired, but it seemed like a good idea to back everything out for now.

Thanks, Raina Masand PCF Release Engineering

concourse-bot commented 8 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.

vito commented 8 years ago

Thanks for the detailed update! Sounds good to me; merging.