Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Refactor EnvironConfigGetter.CloudSpec() #7738
Conversation
mjs
referenced this pull request
Aug 14, 2017
Closed
WIP: remove model tag from EnvironConfigGetter.CloudSpec #7733
| -func NewCloudSpecAPI(facade base.FacadeCaller) *CloudSpecAPI { | ||
| - return &CloudSpecAPI{facade} | ||
| +func NewCloudSpecAPI(facade base.FacadeCaller, modelTag names.ModelTag) *CloudSpecAPI { | ||
| + return &CloudSpecAPI{facade, modelTag} | ||
| } | ||
| // CloudSpec returns the cloud specification for the model | ||
| // with the given tag. |
| @@ -66,6 +65,11 @@ func (c *Client) AllModels() ([]base.UserModel, error) { | ||
| return result, nil | ||
| } | ||
| +func (c *Client) CloudSpec(modelTag names.ModelTag) (environs.CloudSpec, error) { |
| @@ -9,6 +9,7 @@ import ( | ||
| "github.com/juju/juju/api/agent" | ||
| apitesting "github.com/juju/juju/api/testing" | ||
| jujutesting "github.com/juju/juju/juju/testing" | ||
| + jc "github.com/juju/testing/checkers" |
| } | ||
| // NewCloudSpecAPI creates a CloudSpecAPI using the provided | ||
| // FacadeCaller. | ||
| -func NewCloudSpecAPI(facade base.FacadeCaller) *CloudSpecAPI { | ||
| - return &CloudSpecAPI{facade} | ||
| +func NewCloudSpecAPI(facade base.FacadeCaller, modelTag names.ModelTag) *CloudSpecAPI { |
wallyworld
Aug 15, 2017
Owner
I'd personally prefer we pass in a UUID since tags are supposed to be internal to the api/apiserver layers, but given we already leak them everywhere, I guess it doesn't matter.
mjs
Aug 15, 2017
Contributor
yeah I know. it needs a model tag and the caller already has a model tag so passing a model tag seemed the most pragmatic approach.
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jujubot
merged commit 97519d2
into
juju:develop
Aug 15, 2017
1 check passed
continuous-integration/jenkins/pr-merge
This commit looks good
Details
mjs
deleted the
mjs:EnvironConfigGetter-cleanup
branch
Aug 15, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
mjs commentedAug 14, 2017
Description of change
This surprisingly large change updates the
environs.EnvironConfigGetterinterface so theCloudSpecmethod no longer takes a model tag. This means that a single instance of an EnvironConfigGetter is now only ever responsible for a single model.This change is required because it supports changes around
state.Modelcreation. ACloudSpecmethod that takes a model tag encourages poor practices inEnvironConfigGetterimplementations.QA steps
Just confirmed general functionality:
Documentation changes
N.A.
Bug reference
N.A.