Controllers.yaml use file lock. #4347

Merged
merged 2 commits into from Feb 9, 2016

Conversation

Projects
None yet
3 participants
Member

anastasiamac commented Feb 9, 2016

Copied from existing cache.yaml support code which is…heading out.

jujuclient/all.go
@@ -18,3 +36,51 @@ type store struct {
var DefaultControllerStore = func() (ControllerStore, error) {
return &store{}, nil
}
+
+func acquireEnvironmentLock(lockName, operation string) (*fslock.Lock, error) {
@axw

axw Feb 9, 2016

Member

why environment? because copy and paste? if so -- please be intentional, and use meaningful names

@anastasiamac

anastasiamac Feb 9, 2016

Member

bleh ... yes... :(
renamed to lock/unlock \o/

jujuclient/controllers.go
// AllControllers implements ControllersGetter.AllControllers.
// This implementation gets all controllers defined in the controllers file.
func (f *store) AllControllers() (map[string]ControllerDetails, error) {
+ lock, err := acquireEnvironmentLock(controllersLockName, "read-all-controllers")
@axw

axw Feb 9, 2016

Member

a bit verbose; can you please add a store.lock method (and unlock) so we stop repeating things?

Member

axw commented Feb 9, 2016

Couple of nits, otherwise LGTM. Can you please add

TODO(axw) replace with flock on file in $XDG_RUNTIME_DIR

fslock needs to die.

Member

anastasiamac commented Feb 9, 2016

$$merge$$

Contributor

jujubot commented Feb 9, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

jujubot added a commit that referenced this pull request Feb 9, 2016

Merge pull request #4347 from anastasiamac/controllers-file-lock
Controllers.yaml use file lock.

Copied from existing cache.yaml support code which is…heading out.

@jujubot jujubot merged commit 4f4786c into juju:cloud-credentials Feb 9, 2016

@anastasiamac anastasiamac deleted the anastasiamac:controllers-file-lock branch Feb 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment