openconfig / gnmic

gNMIc is a gNMI CLI client and collector
https://gnmic.openconfig.net
Apache License 2.0
179 stars 57 forks source link

--config flag expects strict filename extensions. Error reporting could be improved and document updated #238

Open baijuw opened 1 year ago

baijuw commented 1 year ago

The documentation at https://gnmic.openconfig.net/user_guide/configuration_intro/ says that the configuration file format can be JSON, YAML, TOML, HCL, or Properties. By default, gnmic searches for a file named .gnmic.[yml/yaml, toml, json]. This is understandable for discovering the configuration filename when one is not explicitly called out. However, it is not clear that when the --config flag is used, the filename extension must still end with .yml/yaml, toml, json. Additionally, I was hoping that when the configuration filename is provided with the --config flag, the filename extension would not matter at all. However, as of now, something like the following will fail without a clear error message:

gnmic --config gnmic-config.txt sub
Error: no subscriptions or inputs configuration found

Similarly, if I were to provide a non existing filename, it would still fail with the same message. Preferably a File not found would be more apt here.

gnmic --config thisfileisnotthere sub
Error: no subscriptions or inputs configuration found

The only way this works as of now is if we follow the same filename extensions as expected with the default format [yml/yaml, toml, json].

gnmic --config gnmic-config.yml sub

The gnmic version

version : 0.32.0
 commit : 9168854
   date : 2023-09-05T15:55:35Z
 gitURL : https://github.com/openconfig/gnmic
   docs : https://gnmic.openconfig.net/

I am requesting a few changes:

  1. Ignore filename extension when --config switch is used.
  2. Provide accurate error message when the configuration file is missing with a fileNotFound error.
  3. If filename extension is going to be mandatory even for –config change the error message to indicate expected file format extension not found.
hellt commented 1 year ago

I believe file extensions should stay. They are used by the underlying library to detect the file format to support multiple encodings.

If you want to add some stuff to your file name use file name and don't touch file extension

baijuw commented 1 year ago

If that is the case, then can the error reporting be improved as of now a non existing config also throws the same error as file extension issue.