-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support to manipulate cluster name #104
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package rabbithole | ||
|
||
import ( | ||
"encoding/json" | ||
"net/http" | ||
) | ||
|
||
type ClusterName struct { | ||
Name string `json:"name"` | ||
} | ||
|
||
func (c *Client) GetClusterName() (rec *ClusterName, err error) { | ||
req, err := newGETRequest(c, "cluster-name/") | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
if err = executeAndParseRequest(c, req, &rec); err != nil { | ||
return nil, err | ||
} | ||
|
||
return rec, nil | ||
} | ||
|
||
func (c *Client) SetClusterName(cn ClusterName) (res *http.Response, err error) { | ||
body, err := json.Marshal(cn) | ||
if err != nil { | ||
return nil, err | ||
} | ||
req, err := newRequestWithBody(c, "PUT", "cluster-name", body) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
res, err = executeRequest(c, req) | ||
|
||
return res, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -265,6 +265,27 @@ var _ = Describe("Rabbithole", func() { | |
}) | ||
}) | ||
|
||
Context("PUT /cluster-name", func() { | ||
It("Set cluster name", func() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test does not restore cluster name to its original value. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, no problem. BTW, since cluster name is defined at run time and is hostname based, it may be more elegant and predictable to set arbitrarily a value in ./bin/ci/before_build.sh. Are you ok with that ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup. I don't think we depend on cluster name anywhere. |
||
previousClusterName, err := rmqc.GetClusterName() | ||
Ω(err).Should(BeNil()) | ||
Ω(previousClusterName.Name).Should(Equal("rabbitmq@localhost")) | ||
cnStr := "rabbitmq@rabbit-hole-test" | ||
cn := ClusterName{Name: cnStr} | ||
resp, err := rmqc.SetClusterName(cn) | ||
Ω(err).Should(BeNil()) | ||
Ω(resp.Status).Should(Equal("204 No Content")) | ||
awaitEventPropagation() | ||
cn2, err := rmqc.GetClusterName() | ||
Ω(err).Should(BeNil()) | ||
Ω(cn2.Name).Should(Equal(cnStr)) | ||
// Restore cluster name | ||
rmqc.SetClusterName(*previousClusterName) | ||
Ω(err).Should(BeNil()) | ||
Ω(resp.Status).Should(Equal("204 No Content")) | ||
}) | ||
}) | ||
|
||
Context("GET /connections when there are active connections", func() { | ||
It("returns decoded response", func() { | ||
// this really should be tested with > 1 connection and channel. MK. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this should take a string… or reusing the same struct is actually OK. In most cases I've seen a cluster name is not set to a previously fetched value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially chose to use a string as parameter, but I though passing a struct is more consistent with the API, since cluster name is set via the request body and not as part of path. I've have no problem to switch back to string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gerhard and I discussed this and perhaps using a struct is not too odd. It would be more flexible should this part of the API change or require validation and it's not too weird for Go libraries to do something like this.
So no need for changing it. Thank you for responding so quickly!