Create Logical Device Error : BrokerCLI

[cufffs]

All issues seem to relate to: File: src/python/broker-cly.py (https://github.com/DPIclimate/broker/blob/master/src/python/broker-cli.py)

Issue: per documentation I cannot create logical device due to errors in code, however physical device works fine.

issue #1: When running the below command per documentation: docker exec test-lm-1 python -m broker-cli ld create --json '{ "name": "Test device", "location": { "lat": -39.769108, "long": 147.920245 } }' fails at https://github.com/DPIclimate/broker/blob/816234ef36b6d517b3f07151a785f480432d05af/src/python/broker-cli.py#L274 due to checking for args.ld when it should be args.pd per https://github.com/DPIclimate/broker/blob/816234ef36b6d517b3f07151a785f480432d05af/src/python/broker-cli.py#L73

so line 274 should be: changed to: elif args.cmd2 == 'create' and args.pd is not None:

issue #2: once the above is changed, dict_from_file_or_string() begins to fail due to improper checks. https://github.com/DPIclimate/broker/blob/816234ef36b6d517b3f07151a785f480432d05af/src/python/broker-cli.py#L194

I think due to only checking if arg exists, which they probably always do, but not whether they are None or not, which means the first if is always true, and will cause errors between create physical device and create logical device. I believe lines lines 199 and 206 should be changed to:

199: if args.in_filename is not None: 206:elif args.pd is not None:

after these changes i seem to be able to create logical device, create physical device and map them as intended per documentation.

I have not tested it fully, however TestDAO still passes with OK (no change) however from skimming through the test code the code doesn't test the above as it directly uses the Logical constructor

happy to create PR

Callum from CSU team 3 TSDB

[dajtxx]

Thanks Callum.

I agree with your proposed changes.

There is another bug here too:

Line 274 should check if either args.pd or args.in_filename are set, not just args.pd.

And I should add a comment to say why args.pd is being used rather than args.ld - a convenience so that dict_from_file_or_string doesn't have to check both args.pd and args.ld.

[cufffs]

Cool, in that case I would then suggest that line ~248 be mirrored off 274 as it (physical create) is missing all the checks, though the checks may be redundant as dict_from_file_or_string() will safely raise exceptions with better responses than bypassing it entirely.