theapsgroup / steampipe-plugin-vsphere

Use SQL to instantly query vSphere VMs, networks and more. Open source CLI. No DB required.
https://hub.steampipe.io/plugins/theapsgroup/vsphere
Apache License 2.0
6 stars 5 forks source link

Suggestions for plugin release #5

Closed cbruno10 closed 3 years ago

cbruno10 commented 3 years ago

Hey @Bonemind and @graza-io , thanks for creating this plugin! We've reviewed it and have some suggestions before publishing below. If you have any questions, please let me know, thanks!

index.md:

go.mod

vsphere/utils.go

README.md

Table and column descriptions

Different casing of datastore, also missing periods at the end of each sentence

  > .inspect vsphere.vsphere_datastore
  +-------------+---------+--------------------------------------------------------------------------+
  | column      | type    | description                                                              |
  +-------------+---------+--------------------------------------------------------------------------+
  | accessible  | boolean | Whether this datastore is accessible                                     |
  | capacity    | bigint  | Capacity in bytes                                                        |
  | free        | bigint  | Free space left in bytes                                                 |
  | name        | text    | The name of the datastore                                                |
  | type        | text    | The type of Datastore                                                    |
  | uncommitted | bigint  | How much storage on this datastore has been allocated to guests in bytes |
  +-------------+---------+--------------------------------------------------------------------------+

Typo in the word Curent:

  > .inspect vsphere.vsphere_host
  +--------------+--------+----------------------------------+
  | column       | type   | description                      |
  +--------------+--------+----------------------------------+
  | cpu          | text   | CPU model                        |
  | cpu_cores    | bigint | CPU core count                   |
  | cpu_mhz      | bigint | CPU clock rate in mhz            |
  | cpu_threads  | bigint | CPU thread count                 |
  | cpu_usage    | bigint | Current cpu usage in mhz         |
  | manufacturer | text   | The manufacturer of the hardware |
  | memory       | bigint | Memory in bytes                  |
  | memory_usage | bigint | Curent memory usage in MB        |
  | model        | text   | The model of the hardware        |
  | name         | text   | The host's name                  |
  | num_hbas     | bigint | The number of HBAs connected     |
  | num_nics     | bigint | The number of NICs connected     |
  | status       | text   | The status of the host           |
  | uptime       | bigint | Uptime                           |
  +--------------+--------+----------------------------------+

Are these descriptions pulled from the vSphere SDK or API, or somewhere else? In general, we prefer to use any descriptions from those sources so we can provide users with accurate, easy to find information. Our (short) list of best practices around table and column descriptions can be found at Table and Column Descriptions.

cbruno10 commented 3 years ago

Also, while looking through the table code, the list functions didn't use any paging. Does the SDK support pagination today for some or all of the resource list calls?

graza-io commented 3 years ago

@cbruno10 thanks we'll get on it :)

Bonemind commented 3 years ago

I've merged a pr that should address your points. SDK doesn't seem to support pagination

cbruno10 commented 3 years ago

Thanks @graza-io and @Bonemind !

I believe the plugin is close to ready for publishing, but I had a few suggestions/questions on the config options and docs:

Bonemind commented 3 years ago

Thanks for the feedback. I've renamed the config variables and allowed them to also be set using environment variables. Insecure connection (now allow_unverified_ssl) is just for the validation of the server's ssl certificate, So no additional configuration for users. I've also defaulted insecure connections to false.

cbruno10 commented 3 years ago

Thanks @Bonemind ! I'm closing this issue as v0.0.1 is now released 😃