`DURATION` and `START_TIME` error reporting can be wrong or confusing

llucax commented 3 days ago

What happened?

As reported in #90 and #89, the errors received when a duration is wrong are not clear at all.

Example from #90:

$ dispatch-cli --url grpc://fz-0004.frequenz.io:50052 --key "********" create 174 peak-shaving BATTERY 23:00 00:00
Using API URL: grpc://fz-0004.frequenz.io:50052
Traceback (most recent call last):
  File "/home/daniz/projects/frequenz-client-dispatch-python/.venv/bin/dispatch-cli", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/daniz/projects/frequenz-client-dispatch-python/src/frequenz/client/dispatch/__main__.py", line 469, in main
    asyncio.run(cli.main())
  File "/usr/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 687, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/home/daniz/projects/frequenz-client-dispatch-python/.venv/lib/python3.12/site-packages/asyncclick/core.py", line 1120, in main
    rv = await self.invoke(ctx)
         ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/daniz/projects/frequenz-client-dispatch-python/.venv/lib/python3.12/site-packages/asyncclick/core.py", line 1739, in invoke
    return await _process_result(await sub_ctx.command.invoke(sub_ctx))
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/daniz/projects/frequenz-client-dispatch-python/.venv/lib/python3.12/site-packages/asyncclick/core.py", line 1485, in invoke
    return await ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/daniz/projects/frequenz-client-dispatch-python/.venv/lib/python3.12/site-packages/asyncclick/core.py", line 824, in invoke
    rv = await rv
         ^^^^^^^^
  File "/home/daniz/projects/frequenz-client-dispatch-python/src/frequenz/client/dispatch/__main__.py", line 273, in create
    dispatch = await ctx.obj["client"].create(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/daniz/projects/frequenz-client-dispatch-python/src/frequenz/client/dispatch/_client.py", line 285, in create
    request.to_protobuf(), metadata=self._metadata
    ^^^^^^^^^^^^^^^^^^^^^
  File "/home/daniz/projects/frequenz-client-dispatch-python/src/frequenz/client/dispatch/_internal_types.py", line 118, in to_protobuf
    dispatch_data=DispatchData(
                  ^^^^^^^^^^^^^
ValueError: Value out of range: -79982

What did you expect instead?

Having a clear error message indicating the problem, for example:

$ dispatch-cli --url grpc://fz-0004.frequenz.io:50052 --key "********" create 174 peak-shaving BATTERY 23:00 00:00
Invalid duration "00:00", see --help for details.

For the comment about seeing --help, we need to also implement #91.

Affected version(s)

No response

Affected part(s)

Comnand-line interface (part:cli)

Extra information

No response

llucax commented 3 days ago

A way to cope with this and also have a (probably) more tested alternative would be to use tempora.parse_timedelta to do the parsing.

llucax commented 3 days ago

Cases where START_TIME also gives some not ideal errors:

$ python -m frequenz.client.dispatch --key lasl  create 1 luca-test BATTERY "now + 01:00" 10h
Using API URL: grpc://fz-0004.frequenz.io:50051
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/home/luca/devel/dispatch-client/src/frequenz/client/dispatch/__main__.py", line 473, in <module>
    main()
  File "/home/luca/devel/dispatch-client/src/frequenz/client/dispatch/__main__.py", line 469, in main
    asyncio.run(cli.main())
  File "/home/luca/.local/share/uv/python/cpython-3.11.10-linux-x86_64-gnu/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/home/luca/.local/share/uv/python/cpython-3.11.10-linux-x86_64-gnu/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/luca/.local/share/uv/python/cpython-3.11.10-linux-x86_64-gnu/lib/python3.11/asyncio/base_events.py", line 654, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/home/luca/devel/dispatch-client/.direnv/python-3.11/lib/python3.11/site-packages/asyncclick/core.py", line 1120, in main
    rv = await self.invoke(ctx)
         ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/luca/devel/dispatch-client/.direnv/python-3.11/lib/python3.11/site-packages/asyncclick/core.py", line 1739, in invoke
    return await _process_result(await sub_ctx.command.invoke(sub_ctx))
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/luca/devel/dispatch-client/.direnv/python-3.11/lib/python3.11/site-packages/asyncclick/core.py", line 1485, in invoke
    return await ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/luca/devel/dispatch-client/.direnv/python-3.11/lib/python3.11/site-packages/asyncclick/core.py", line 824, in invoke
    rv = await rv
         ^^^^^^^^
  File "/home/luca/devel/dispatch-client/src/frequenz/client/dispatch/__main__.py", line 273, in create
    dispatch = await ctx.obj["client"].create(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/luca/devel/dispatch-client/src/frequenz/client/dispatch/_client.py", line 264, in create
    raise ValueError("start_time must not be in the past")
ValueError: start_time must not be in the past

Marenz commented 3 days ago

Hmm the last example should work

frequenz-floss / frequenz-client-dispatch-python