You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by Graeme-Miller <gi...@git.apache.org> on 2017/06/12 13:26:27 UTC
[GitHub] brooklyn-client pull request #54: Fixed setting config. Added setting sensor...
GitHub user Graeme-Miller opened a pull request:
https://github.com/apache/brooklyn-client/pull/54
Fixed setting config. Added setting sensors
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Graeme-Miller/brooklyn-client set_config_sensor
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-client/pull/54.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #54
----
commit 34382b09fbb8d0bd3d4b1f3cc6c4d139df7ab8c7
Author: graeme.miller <gr...@cloudsoftcorp.com>
Date: 2017-06-12T13:20:45Z
Fixed setting config. Added setting sensors
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #54: Fixed setting config. Added setting sensor...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/54#discussion_r121912681
--- Diff: cli/api/entity_config/config.go ---
@@ -52,7 +52,7 @@ func ConfigValueAsBytes(network *net.Network, application, entity, config string
func SetConfig(network *net.Network, application, entity, config, value string) (string, error) {
url := fmt.Sprintf("/v1/applications/%s/entities/%s/config/%s", application, entity, config)
- val := []byte(value)
+ val := []byte("\""+value+"\"")
--- End diff --
This will prevent setting json containers/numbers. Currently you can set both `'{key: 1}'` and `'"mystring"'`.
* Should we add an option which indicates the value is json?
* Do we care about backwards compatibility where someone could already be setting strings with the quotes added?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #54: Fixed setting config. Added setting sensors
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/54
@Graeme-Miller should this PR be closed now in light of the above conversation?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #54: Fixed setting config. Added setting sensor...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/54#discussion_r121902650
--- Diff: cli/commands/set.go ---
@@ -51,9 +53,28 @@ func (cmd *SetConfig) Run(scope scope.Scope, c *cli.Context) {
if err := net.VerifyLoginURL(cmd.network); err != nil {
error_handler.ErrorExit(err)
}
- response, err := entity_config.SetConfig(cmd.network, scope.Application, scope.Entity, scope.Config, c.Args().First())
- if nil != err {
- error_handler.ErrorExit(err)
+
+ config := scope.Config
+ sensor := scope.Sensor
+ if config != "" && sensor != "" {
+ error_handler.ErrorExit(errors.New("Both config and sensor supplied when calling set. Please only specify one."))
+ }
+
+ if config == "" && sensor == "" {
+ error_handler.ErrorExit(errors.New("Please specify either config or scope when calling set."))
--- End diff --
"config or sensor"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #54: Fixed setting config. Added setting sensors
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/54
@Graeme-Miller sounds good to me
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #54: Fixed setting config. Added setting sensor...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/54#discussion_r121903310
--- Diff: cli/commands/set.go ---
@@ -51,9 +53,28 @@ func (cmd *SetConfig) Run(scope scope.Scope, c *cli.Context) {
if err := net.VerifyLoginURL(cmd.network); err != nil {
error_handler.ErrorExit(err)
}
- response, err := entity_config.SetConfig(cmd.network, scope.Application, scope.Entity, scope.Config, c.Args().First())
- if nil != err {
- error_handler.ErrorExit(err)
+
+ config := scope.Config
+ sensor := scope.Sensor
+ if config != "" && sensor != "" {
+ error_handler.ErrorExit(errors.New("Both config and sensor supplied when calling set. Please only specify one."))
+ }
+
+ if config == "" && sensor == "" {
+ error_handler.ErrorExit(errors.New("Please specify either config or scope when calling set."))
+ }
+
+ if config != "" {
+ _, err := entity_config.SetConfig(cmd.network, scope.Application, scope.Entity, config, c.Args().First())
+ if nil != err {
+ error_handler.ErrorExit(err)
+ }
+ fmt.Println("Config set correctly")
--- End diff --
Just leave this line out, and 78 below.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #54: Fixed setting config. Added setting sensors
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/54
Taking @neykov's point about backward compatiblity I'll reverse my suggestion above and say that `set` (with no args) remains as-is, while, say, `set -s` (s for string) is for auto-quoting the value as a string.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #54: Fixed setting config. Added setting sensor...
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller closed the pull request at:
https://github.com/apache/brooklyn-client/pull/54
---
[GitHub] brooklyn-client pull request #54: Fixed setting config. Added setting sensor...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/54#discussion_r121940013
--- Diff: cli/api/entity_config/config.go ---
@@ -52,7 +52,7 @@ func ConfigValueAsBytes(network *net.Network, application, entity, config string
func SetConfig(network *net.Network, application, entity, config, value string) (string, error) {
url := fmt.Sprintf("/v1/applications/%s/entities/%s/config/%s", application, entity, config)
- val := []byte(value)
+ val := []byte("\""+value+"\"")
--- End diff --
I'd say we do care about backwards compatibility. I'll reverse my earlier suggestion about defaults. Will update the comment...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #54: Fixed setting config. Added setting sensors
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on the issue:
https://github.com/apache/brooklyn-client/pull/54
Actually, maybe all that is needed is some clarity about how to use this in the help documentation and some better error information.
I was fixing this because of this Jira (https://issues.apache.org/jira/browse/BROOKLYN-465) and I also found using set confusing.
How would you both feel about me updating the help documentation for set to say it is JSON, and to quote strings? I'll also leave in the changes to allow sensors to be set.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #54: Fixed setting config. Added setting sensors
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/brooklyn-client/pull/54
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/brooklyn-client-pull-requests/139/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #54: Fixed setting config. Added setting sensors
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on the issue:
https://github.com/apache/brooklyn-client/pull/54
Closed, this solution doesnt fix the issue
---