-
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
Basic support for definition export #170
Conversation
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.
Thank you for taking the time to contribute.
I'm afraid using interfaces for most entity collections is not something we would be interested in doing. This significantly reduces the amount of effort involved but makes it very hard to reason about the structure of responses.
You can use Hop as an example of a library that supports definition import and export in a more type-safe manner.
I'd understand if you decide against spending more time on this PR.
Hey,
Would it be ok if I replace all interfaces for specific types? I can improve it
…On Tue, 22 Dec 2020 at 22:33 Michael Klishin ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Thank you for taking the time to contribute.
I'm afraid using interfaces for most entity collections is not something
we would be interested in doing. This significantly reduces the amount of
effort involved but makes it very hard to reason about the structure of
responses.
You can use Hop <https://github.com/rabbitmq/hop> as an example of a
library that supports definition import and export in a more type-safe
manner.
I'd understand if you decide against spending more time on this PR.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#170 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4YEQGLSEVY3M6C64HBZPTSWEGB3ANCNFSM4VEWE6VQ>
.
|
@pathcl sure, that's what I had in mind. Thank you. Can we please introduce a new type for global parameters? I can help get it over the finish line do it if you are not familiar with them. |
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.
This is definitely a step in the right direction. Let's also cover global parameters.
@@ -16,9 +16,44 @@ type RuntimeParameter struct { | |||
Value interface{} `json:"value"` | |||
} | |||
|
|||
// RuntimeDefinitions represents rabbitmq runtime configuration. | |||
type RuntimeDefinitions struct { |
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.
They are called just definitions, not runtime definitions. A lot of them have nothing to do with runtime parameters and do not change at runtime (e.g. product version).
// GET /api/definitions | ||
// | ||
|
||
func (c *Client) ListRuntimeDefinitions() (p *RuntimeDefinitions, err error) { |
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.
This should be renamed per the comment above.
Finished this and merged manually. Thank you. We may want to compare the global runtime parameters serialization to that of Hop but I'm reasonably confident in this feature as is. |
Hello,
Nice library! thanks for making it open source. I tried to find a way to get whole rabbitmq configuration but couldn't find it.
Idea here is to get full config from rabbitmq runtime. Eventually this could be used along with terraform to eventually import/manage it.