Allow passing of json argument to service call #319

glance- · 2020-01-14T09:47:51Z

When calling service who needs arguments which are other things than
strings, ex nested dicts, its easiest done by being able to pass a json
string as argument, for example when calling html5.dismis:

hass-cli service call html5.dismiss --arguments-json '{"target": "DEVICE", "data": {"tag": "TAG"}}'

Signed-off-by: Anton Lundin glance@acc.umu.se

homeassistant_cli/plugins/service.py

glance- · 2020-01-14T09:50:03Z

This PR is a bit quick and dirty and is to be treated as a RFC, but it shows the issue I'm trying to solve, and a simple fix for it.

Please comment about other ways you might want to tackle this issue.

maxandersen · 2020-01-14T10:17:01Z

I grok the incentive, initially I actually wanted the --arguments / to_arguments() function to support nested structures but didn't get around to it.

but if we are going to allow json we should consider supporting yaml too and possibly as a piped/redirected format. That should be easy to do as one is a subset of the other; maybe just have --arguments-raw or --arguments-data take something that will be loaded with yaml load always ?

--arguments-raw=- would be read from stdin
--arguments-raw=@filename would read from file
--arguments-raw='{"target": "DEVICE", "data": {"tag": "TAG"}}'

wdyt?

glance- · 2020-01-14T11:32:01Z

--arg=- for stdin and --arg=@filename for read data from file is ok for me, and semi-standard and what ex. curl uses, so I'm just fine with that.

--arguments-raw sort of makes sense, and is what the underlying HA api uses, but I really don't think we need to expose that to the user. It also turns things into a guessing game if its yaml the user provides, or if its json.

I'd rather go for --arguments-json and --arguments-yaml and let the user be explicit about that it provides, and let the different options be conflicting, so the tool errors out if you provide more than one of --arguments, --arguments-json or --arguments-yaml.

I think json is way easier to write on cmdline than yaml, but I much rather write yaml in a file and provide that than writing json in a file. With some sane design of the code we can support all the permutations and let the user decide what they like.

maxandersen · 2020-01-16T16:13:44Z

makes sense! Would you like to update your PR to get that done ?

glance- · 2020-01-16T21:27:27Z

Just saw that hass-cli raw ws takes a --json=... and it would be nice to try to keep things consistent. That would argue for calling the args --json and --yaml. Do that feel better than --arguments-json and --arguments-yaml ?

I'll hammer out a variant. It's easy to change the name later, before merging this.

maxandersen · 2020-01-17T10:49:08Z

good point! +1 for consistency.

tests/test_service.py

homeassistant_cli/plugins/service.py

tests/test_service.py

glance- · 2020-01-21T09:43:58Z

hurm. Only one of the typing issues is "mine", and I'm sort of confused by how that code is written.

homeassistant_cli.yaml.dumpyaml takes a YAML, but doesn't use it. Sure, I can workaround it by just creating a YAML-object and letting it get thrown away:

diff --git i/tests/test_service.py w/tests/test_service.py
index bb7b52f..447afb2 100644
--- i/tests/test_service.py
+++ w/tests/test_service.py
@@ -8,7 +8,7 @@ import requests_mock
 import homeassistant_cli.autocompletion as autocompletion
 import homeassistant_cli.cli as cli
 from homeassistant_cli.config import Configuration
-from homeassistant_cli.yaml import dumpyaml
+from homeassistant_cli.yaml import yaml, dumpyaml
 
 
 def test_service_list(default_services) -> None:
@@ -218,7 +218,7 @@ def test_service_call_with_yaml_file(default_services) -> None:
             "test": True,
         }
 
-        open_yaml_file = mock_open(read_data=dumpyaml(None, data=data))
+        open_yaml_file = mock_open(read_data=dumpyaml(yaml(), data=data))
 
         runner = CliRunner()
         with patch('builtins.open', open_yaml_file) as mocked_open:

Would you like me to do that, or should we fix the dumpyaml function instead?

maxandersen · 2020-01-21T12:51:02Z

do the fix you think is right.

codecov-io · 2020-01-21T13:30:00Z

Codecov Report

Merging #319 into dev will increase coverage by 0.18%.
The diff coverage is 86.84%.

@@            Coverage Diff             @@
##              dev     #319      +/-   ##
==========================================
+ Coverage   75.25%   75.43%   +0.18%     
==========================================
  Files          22       22              
  Lines        1560     1592      +32     
==========================================
+ Hits         1174     1201      +27     
- Misses        386      391       +5

Impacted Files	Coverage Δ
homeassistant_cli/helper.py	`74.22% <100%> (+0.26%)`	⬆️
homeassistant_cli/cli.py	`76.13% <100%> (ø)`	⬆️
homeassistant_cli/plugins/service.py	`92.7% <85.71%> (-4.22%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1528c0a...8f6b8a6. Read the comment docs.

glance- · 2020-01-21T14:37:11Z

The current typing failure is in ruamel.yaml==0.16.6 which was released yesterday (2020-01-20) and is definitely out of scope for this PR.

stale · 2020-03-23T12:40:55Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

houndci-bot · 2020-04-04T19:26:45Z

homeassistant_cli/plugins/service.py

+    '--yaml', help="""Yaml string to use as arguments.
+if string is -, the data is read from stdin, and if it starts with the letter @
+the rest should be a filename from which the data is read""",
+    callback=argument_callback,


undefined name 'argument_callback'

houndci-bot · 2020-04-04T19:26:46Z

homeassistant_cli/plugins/service.py

+    '--json', help="""Json string to use as arguments.
+if string is -, the data is read from stdin, and if it starts with the letter @
+the rest should be a filename from which the data is read""",
+    callback=argument_callback,


undefined name 'argument_callback'

houndci-bot · 2020-04-04T19:26:46Z

homeassistant_cli/plugins/service.py

+    '--arguments', help="""Comma separated key/value pairs to use as arguments.
+if string is -, the data is read from stdin, and if it starts with the letter @
+the rest should be a filename from which the data is read""",
+    callback=argument_callback,


undefined name 'argument_callback'

houndci-bot · 2020-04-04T19:26:46Z

homeassistant_cli/plugins/service.py

@@ -11,6 +13,7 @@
 from homeassistant_cli.config import Configuration
 from homeassistant_cli.helper import format_output, to_attributes
 import homeassistant_cli.remote as api
+from homeassistant_cli.yaml import yaml


'homeassistant_cli.yaml.yaml' imported but unused

houndci-bot · 2020-04-04T19:26:46Z

homeassistant_cli/plugins/service.py

@@ -3,6 +3,8 @@
 import re as reg
 import sys
 from typing import Any, Dict, List, Pattern  # noqa: F401
+import json
+import io


'io' imported but unused

houndci-bot · 2020-04-04T19:26:46Z

homeassistant_cli/plugins/service.py

@@ -3,6 +3,8 @@
 import re as reg
 import sys
 from typing import Any, Dict, List, Pattern  # noqa: F401
+import json


'json' imported but unused

houndci-bot · 2020-04-04T19:26:47Z

homeassistant_cli/plugins/raw.py

 @pass_context
-def post(ctx: Configuration, method, json):
+def post(ctx: Configuration, method, data={}):


Do not use mutable data structures for argument defaults. They are created during function definition time. All calls to the function reuse this one instance of that data structure, persisting changes between them.

houndci-bot · 2020-04-04T19:26:47Z

homeassistant_cli/helper.py

+        _LOGGING.error("Parameter name is unknown: %s", param.name)
+        ctx.exit()
+
+    if isinstance(value, io.IOBase):


undefined name 'io'

houndci-bot · 2020-04-04T19:26:47Z

homeassistant_cli/helper.py

+
+    if value == '-':  # read from stdin
+        _LOGGING.debug("Loading value from stdin")
+        value = sys.stdin


undefined name 'sys'

glance- · 2021-12-06T22:39:29Z

I've just refreshed this one, could someone take a look at it and hopefully merge it?

@filename

When calling service who needs arguments which are other things than strings, ex nested dicts, its easiest done by being able to pass a json or yaml argument, for example when calling html5.dismis: hass-cli service call html5.dismiss --json '{"target": "DEVICE", "data": {"tag": "TAG"}}' As requested in review, this also adds support for passing in @filename or - for reading arguments from stdin or from a file. Signed-off-by: Anton Lundin <glance@acc.umu.se>

This is to simplify re-use outside service calls.

This re-uses the --yaml and --json argument parsers from services to allow passing from stdin and files. One awesome use case is: hass-cli raw ws lovelace/config > ui-lovelace.yaml $EDITOR ui-lovelace.yaml , replacing the status fields, up until result: with config:, and then, hass-cli raw ws lovelace/config/save --yaml @ui-lovelace.yaml

homeassistant added the cla-signed label Jan 14, 2020

houndci-bot reviewed Jan 14, 2020

View reviewed changes

homeassistant_cli/plugins/service.py Outdated Show resolved Hide resolved

glance- force-pushed the service-call-json branch from 74ff2a9 to 10cade8 Compare January 14, 2020 09:53

maxandersen mentioned this pull request Jan 17, 2020

How to pass complex arguments to hass-cli #320

Closed

glance- force-pushed the service-call-json branch from 10cade8 to 3aeca23 Compare January 21, 2020 09:13

houndci-bot reviewed Jan 21, 2020

View reviewed changes

glance- force-pushed the service-call-json branch from 3aeca23 to cb0fe08 Compare January 21, 2020 09:16

houndci-bot reviewed Jan 21, 2020

View reviewed changes

tests/test_service.py Show resolved Hide resolved

glance- force-pushed the service-call-json branch 2 times, most recently from 4fb74dd to a3e7560 Compare January 21, 2020 09:21

glance- force-pushed the service-call-json branch from a3e7560 to b66a3bc Compare January 21, 2020 13:27

glance- force-pushed the service-call-json branch from b66a3bc to 8f6b8a6 Compare January 21, 2020 14:05

glance- requested a review from maxandersen January 23, 2020 11:46

stale bot added the wontfix label Mar 23, 2020

maxandersen added the pinned label Mar 23, 2020

stale bot removed the wontfix label Mar 23, 2020

glance- force-pushed the service-call-json branch from 8f6b8a6 to cc527c0 Compare April 4, 2020 19:26

houndci-bot reviewed Apr 4, 2020

View reviewed changes

glance- force-pushed the service-call-json branch from cc527c0 to 2788a8a Compare April 4, 2020 21:13

glance- force-pushed the service-call-json branch from 2788a8a to 9e21b96 Compare December 6, 2021 22:34

kreucher mentioned this pull request Jan 3, 2023

Issue escaping spaces #351

Open

glance- added 4 commits July 18, 2023 22:24

Correct mypy warning

71c78bb

Move argument parsing callback to helper

816e90f

This is to simplify re-use outside service calls.

glance- force-pushed the service-call-json branch from 9e21b96 to ee7e69f Compare July 22, 2023 17:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow passing of json argument to service call #319

Allow passing of json argument to service call #319

glance- commented Jan 14, 2020

glance- commented Jan 14, 2020

maxandersen commented Jan 14, 2020

glance- commented Jan 14, 2020

maxandersen commented Jan 16, 2020

glance- commented Jan 16, 2020

maxandersen commented Jan 17, 2020

glance- commented Jan 21, 2020

maxandersen commented Jan 21, 2020

codecov-io commented Jan 21, 2020 •

edited

glance- commented Jan 21, 2020

stale bot commented Mar 23, 2020

houndci-bot Apr 4, 2020

houndci-bot Apr 4, 2020

houndci-bot Apr 4, 2020

houndci-bot Apr 4, 2020

houndci-bot Apr 4, 2020

houndci-bot Apr 4, 2020

houndci-bot Apr 4, 2020

houndci-bot Apr 4, 2020

houndci-bot Apr 4, 2020

glance- commented Dec 6, 2021

Allow passing of json argument to service call #319

Are you sure you want to change the base?

Allow passing of json argument to service call #319

Conversation

glance- commented Jan 14, 2020

glance- commented Jan 14, 2020

maxandersen commented Jan 14, 2020

glance- commented Jan 14, 2020

maxandersen commented Jan 16, 2020

glance- commented Jan 16, 2020

maxandersen commented Jan 17, 2020

glance- commented Jan 21, 2020

maxandersen commented Jan 21, 2020

codecov-io commented Jan 21, 2020 • edited

Codecov Report

glance- commented Jan 21, 2020

stale bot commented Mar 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glance- commented Dec 6, 2021

codecov-io commented Jan 21, 2020 •

edited