Add support for default pools for env providers #9

Merged
merged 2 commits into from Jan 27, 2015

Conversation

Projects
None yet
2 participants
Owner

wallyworld commented Jan 27, 2015

There's now a registry of default pools for environment types.
If storage is specified without a pool, the default is used.

state/storage.go
- conf, err := st.EnvironConfig()
- if err != nil {
- return errors.Trace(err)
+ conf, err := st.EnvironConfig()
@axw

axw Jan 27, 2015

Collaborator

This looks fine for now, though I think we should rework this later so that state is not calling into the storage package. It would be better if we made these sorts of decisions in the API server code. Validation is tricky, and probably means extending the policy code.

@wallyworld

wallyworld Jan 27, 2015

Owner

Agreed. Rework definitely needed. I felt dirty for sure, but it was expedient.

state/storage.go
+ )
+ }
+ }
+ if cons.Pool != "" {
@axw

axw Jan 27, 2015

Collaborator

I think cons.Pool is guaranteed to be != "" now ?

@wallyworld

wallyworld Jan 27, 2015

Owner

True, fixed. I did leave it there for a reason but can't recall why now. Must have been a crap reason.

@@ -69,3 +69,25 @@ func IsProviderSupported(envType string, providerType ProviderType) bool {
}
return false
}
+
+type defaultStoragePool map[StorageKind]string
@axw

axw Jan 27, 2015

Collaborator

Doesn't really matter, but I feel compelled to say that use of a map is overkill when there can be at most 2 entries :)

@wallyworld

wallyworld Jan 27, 2015

Owner

Yeah, I can see that point. Was easy to code though :-)

Collaborator

axw commented Jan 27, 2015

Possible simplification re cons.Pool != "" check, but otherwise LGTM.

wallyworld added a commit that referenced this pull request Jan 27, 2015

Merge pull request #9 from wallyworld/default-provider-pools
Add support for default pools for env providers

@wallyworld wallyworld merged commit f839b7a into storage-feature Jan 27, 2015

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