TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
975 stars 306 forks source link

Storage integration CLI errors #3584

Closed KrishnaIyer closed 3 years ago

KrishnaIyer commented 3 years ago

Summary

There are a couple of errors from the CLI when using the storage integration

Steps to Reproduce

Item 1;

 tti-lw-cli apps packages default-associations set "bat-test" 100 --package-name storage-integration

Gives the following warning

  WARN Package data not available               error=error:cmd/ttn-lw-cli/commands:no_data (no data for `data`) name=data

Item 2:

tti-lw-cli apps storage get "bat-test" --limit 10 --after "2018-08-20T00:00:00Z"

This errors

parsing time "2018-08-20T00:00:00Z" as "2006-01-02 15:04:05": cannot parse "T00:00:00Z" as " "

What do you see now?

Warnings and errors as mentioned above.

What do you want to see instead?

No warning/error

Environment

TTS Cloud v3.10.4

How do you propose to implement this?

Item 1:

Item 2:

The docs use ISO 8601 format where as the CLI is parsing UTC. https://thethingsstack.io/integrations/storage/retrieve/ https://github.com/TheThingsNetwork/lorawan-stack/blob/535aafd0db07f70a6fff6621fc9030a8bb3e3bfb/cmd/ttn-lw-cli/commands/flags.go#L322

I've added an update to the docs here. If this is not the case and the code is parsing the wrong format, please fix that.

How do you propose to test this?

Run the CLI and test.

Can you do this yourself and submit a Pull Request?

Item 2 is already fixed here.

Item 1 under consultation from @neoaggelos

neoaggelos commented 3 years ago

For the warning message, this is common for application packages. A strictly correct solution would be to verify the package data against the individual application package, which is too much work for no real gain.

Could we simply drop the message, or add a switch and display only for application packages where it is relevant? @adriansmares

adriansmares commented 3 years ago

For the warning message, this is common for application packages. A strictly correct solution would be to verify the package data against the individual application package, which is too much work for no real gain.

Could we simply drop the message, or add a switch and display only for application packages where it is relevant? @adriansmares

Having a map[string]struct{} that signifies if a specific package (by name) needs data or not is fine by me. At the time of writing this applies only to the LoRaCloud DMS v1 package. Don't forget to update this for both associations and default-associations.