Initial suggestions for plugin release

[madhushreeray30]

Thanks @emeric-martineau and @octo-topi for this new plugin. Great work 🎉 !!

The basic structure looks good so far. While using the plugin, we did come up with a few suggestions based on our best practices:

config/metabase.spc

• Could you please update the metabase.spc file to follow the format of the namecheap plugin
• The user, password, and token should be commented out and the values should be in the format of the actual values (however incorrect)
• Also mention which are the required parameters
• Please add details on how to authenticate with the environment variables

docs/tables/*

• The example queries in the doc look a bit light. Could you please add 2-3 more example queries per table?
• One suggestion would be to reflect more column names instead of just the name or description in all the example queries, this would give a better idea of the displayed entity.
• Another suggestion would be to prevent the usage of select * from table_name instead we can display the important columns
• Examples around JSON columns are also a good way to indicate the usage of the tables

index.md

• The example query should end with a ;
• Also please make sure that the Configuration section is in sync with the changes suggested for the config/metabase.spc file.
• Please update the section for environment variables as follows Instead of > The Metabase plugin will use the following environment variables to obtain credentials **only if other argument (tokenoruserorpassword) is not specified** in the connection: use > Alternatively, you can also use the standard Metabase environment variables to obtain credentials **only if other arguments (tokenoruserorpassword) are not specified** in the connection:
• Also provide the environment variable values similar to the actual format
• Please update the Get Involved section following the namecheap plugin

Table: metabase_db

• Please remove any commented code
• Some columns have a description as ??? we can add any suitable description if not available in API documents
• The list call should be named as listDatabases instead of listDatabase and thus also update the logger messages accordingly
• Why have we missed out the details column?

Table:metabase_db_feature

• The logger message is incorrect and should be changed to plugin.Logger(ctx).Error("metabase_db_feature.listDatabaseFeatures", "connection_error", err) instead of plugin.Logger(ctx).Error("metabase_db.getDatabaseFeatures", "connection_error", err). Also please extend these changes elsewhere.

Table:metabase_table

• Some columns have a description as ??? we can add any suitable description if not available in API documents
• The logger message is incorrect and should be changed to include correct table and function name for instance plugin.Logger(ctx).Error("metabase_db_table.listDatabaseTables", "connection_error", err)
• The list call should be renamed to listDatabaseTables

plugin.go

• How are we handling the not found error?
• Please remove any commented code
• Formatting is incorrect could you please update it

go.mod

• Please update the steampipe SDK version to v5.5.0

README

• The file looks incomplete with many major sections missing. Could you please update it following the format of the namecheap plugin

CHANGELOG

• Could you please write an initial changelog for release for instance >

  
  ## v0.0.1 [TBD]

What's new?

• New tables added
  • metabase_db
  • metabase_db_feature
  • metabase_db_table

A couple of follow-up questions:

How are we handling the resource not found errors? Do the APIs support pagination? I did not find any reference to it, but I want to make sure that we have checked that box before releasing it. While working with the APIs did you encounter rate limit errors? Please refer https://steampipe.io/docs/develop/plugin-release-checklist#data-ingestion for more information Please let us know if you have questions, happy to help 👍.

[emeric-martineau]

Hi @madhushreeray30,

this plugin is still in progress developpement.

Your comments are very helpfull to help us, make better plugin :pray:

We let this issue open until we fix all your suggestion.

:heart_eyes:

[emeric-martineau]

config/metabase.spc

:heavy_check_mark: Could you please update the metabase.spc file to follow the format of the namecheap plugin :heavy_check_mark: The user, password, and token should be commented out and the values should be in the format of the actual values (however incorrect) :heavy_check_mark: Also mention which are the required parameters :heavy_check_mark: Please add details on how to authenticate with the environment variables

docs/tables/*

:heavy_check_mark: The example queries in the doc look a bit light. Could you please add 2-3 more example queries per table? :heavy_check_mark: One suggestion would be to reflect more column names instead of just the name or description in all the example queries, this would give a better idea of the displayed entity. :heavy_check_mark: Another suggestion would be to prevent the usage of select * from table_name instead we can display the important columns :heavy_check_mark: Examples around JSON columns are also a good way to indicate the usage of the tables

index.md

:heavy_check_mark: The example query should end with a ; :heavy_check_mark: Also please make sure that the Configuration section is in sync with the changes suggested for the config/metabase.spc file. :heavy_check_mark: Please update the section for environment variables as follows Instead of > The Metabase plugin will use the following environment variables to obtain credentials **only if other argument (tokenoruserorpassword) is not specified** in the connection: use > Alternatively, you can also use the standard Metabase environment variables to obtain credentials **only if other arguments (tokenoruserorpassword) are not specified** in the connection: :heavy_check_mark: Also provide the environment variable values similar to the actual format :heavy_check_mark: Please update the Get Involved section following the namecheap plugin

Table: metabase_db

:heavy_check_mark: Please remove any commented code :heavy_check_mark: Some columns have a description as ??? we can add any suitable description if not available in API documents :heavy_check_mark: The list call should be named as listDatabases instead of listDatabase and thus also update the logger messages accordingly :heavy_check_mark: Why have we missed out the details column?

Table:metabase_db_feature

:heavy_check_mark: The logger message is incorrect and should be changed to plugin.Logger(ctx).Error("metabase_db_feature.listDatabaseFeatures", "connection_error", err) instead of plugin.Logger(ctx).Error("metabase_db.getDatabaseFeatures", "connection_error", err). Also please extend these changes elsewhere.

Table:metabase_table

:heavy_check_mark: Some columns have a description as ??? we can add any suitable description if not available in API documents :heavy_check_mark: The logger message is incorrect and should be changed to include correct table and function name for instance plugin.Logger(ctx).Error("metabase_db_table.listDatabaseTables", "connection_error", err) :heavy_check_mark: The list call should be renamed to listDatabaseTables

plugin.go

:heavy_check_mark: How are we handling the not found error? :heavy_check_mark: Please remove any commented code :heavy_check_mark: Formatting is incorrect could you please update it

go.mod

:heavy_check_mark: Please update the steampipe SDK version to v5.5.0

README

:heavy_check_mark: The file looks incomplete with many major sections missing. Could you please update it following the format of the namecheap plugin

CHANGELOG

:heavy_check_mark: Could you please write an initial changelog for release for instance >

## v0.0.1 [TBD]

_What's new?_

- New tables added
  - [metabase_db](https://hub.steampipe.io/plugins/1024pix/metabase/tables/metabase_db)
  - [metabase_db_feature](https://hub.steampipe.io/plugins/1024pix/metabase/tables/metabase_db_feature)
  - [metabase_db_table](https://hub.steampipe.io/plugins/1024pix/metabase/tables/metabase_db_table)

[emeric-martineau]

@madhushreeray30 all items are fixed an commited on main branch.

[madhushreeray30]

@emeric-martineau thanks a lot, I shall take a quick look at them and let you know if any further changes are required.

[madhushreeray30]

@emeric-martineau Thanks for the updates, few changes that would be required are listed here:

• README.md

  connection "metabase" {
  plugin          = "1024pix/metabase"
  
  url             = "https://localhost"
  
  # Authentication information
  user            = "janedoe"
  password        = "my_password"
  
  tls_skip_verify = true
  }

  This section should be formatted correctly and it should be in sync with the metabase.spc file. Suggested change would be >

  connection "metabase" {
  plugin = "1024pix/metabase"
  
  # Authentication information
  url = "https://localhost"
  
  # Username/password is required for requests. Required except if token (see after) is provided.
  user = "my_user"
  password = "my_password"
  
  # Token is required for requests. Required except if user/password (see before) is provided.
  token = "33d0d62a-6a16-3083-ba7b-3bab31bd6612"
  
  # Skip TLS verification, useful in local test. Optional.
  tls_skip_verify = false
  }

  • The export commands for the environment variable should not include values in quotes For instance >

    export METABASE_USER=janedoe
    export METABASE_PASSWORD=my_password

• Please keep the queries in lower case For instance >

  select
  id,
  name
  from
  metabase_db;

• index.md

• Please keep the queries in lower case For instance >

  select
  id,
  name
  from
  metabase_db;

• The credential section can be updated to >

  Metabase requires a [token](https://www.metabase.com/learn/administration/metabase-api#authenticate-your-requests-with-a-session-token) or a login username and password.  

• The configuration section should be in sync with the metabase.spc file

  • The token value should be like the actual value however incorrect

• The export commands should not contain values in quotes

  export METABASE_TOKEN=33d0d62a-6a16-3083-ba7b-3bab31bd6612
  export METABASE_USER=my-user
  export METABASE_PASSWORD=my-password

-table docs

• The queries should be in lower case (keywords such as select, from, etc)
• We do not keep the output of the queries, is there any specific reason to keep them?

• table_metabase_permission_data.go
  • Can you update the description from Table id. to Table ID.

As this plugin is still in progress we would encourage you to follow our standards in the upcoming additions to this table and let us know if any questions arise. Thank you!