Initial suggestions for plugin release

[madhushreeray30]

Thanks @jreyesr 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:

postgres.spc

• Could you please update the postgres.spc file to follow the format of the namecheap plugin
• We should mention the config arguments however commented out and the values should be in the format of the actual values (however incorrect)

• For instance: There are only a few formatting issues that have been resolved in this example:

  connection "postgres" {
  plugin = "jreyesr/postgres"
  
    # A connection string (https://pkg.go.dev/github.com/jackc/pgx/v5#hdr-Establishing_a_Connection), in the form that is 
    expected by the pgx package. Required. 
    # Can also be set with the `DATABASE_URL` environment variable.
    # connection_string = "postgres://username:password@localhost:5432/database_name"
  
    # The remote DB's schema that this plugin will expose. If you leave this unset, it'll default to `public`.
    # schema = "public"
  
    # List of tables that will be exposed from the remote DB. No dynamic tables will be created if this arg is empty or not set.
    # Wildcard-based searches are supported.
    # For example:
    #  - "*" will expose every table in the remote DB
    #  - "auth-*" will expose tables whose names start with "auth-"
    #  - "users" will only expose the specific table "users"
    # You can have several items (for example, ["auth-*", "users"] will expose 
    # all the tables that start with "auth-", PLUS the table "users")
    # Defaults to all tables
    # tables_to_expose = ["*"]
  }

• Is the environment variable name correct?

docs/tables

• The table name should be represented like # Table: raw.
• The formatting of the query should be updated using https://www.freeformatter.com/sql-formatter.html#before-output (2 indent spaces)
• The queries are not correctly formatted. They should end with a ;. Please refer to the following query:

  select 
  name,
  id 
  from
  aws_s3_bucket
  where 
  region = 'us-east-1';

• We generally do not include the example query output in the docs, is there a particular reason for keeping them here?
• The example queries in the doc look a bit light. Could you please add 2 to 3 more example queries wherever possible?

docs/index.md

• The description should be updated to "description: Steampipe plugin for proxying queries to plain Postgres databases."
• The og_description should be updated to "Query any Postgres table from Steampipe with SQL! Open source CLI. No DB required."
• The formatting of the query should be updated using https://www.freeformatter.com/sql-formatter.html#before-output (2 indent spaces)
• The query is not correctly formatted. They should end with a ;.
• We generally do not include the example query output in the docs, is there a particular reason for keeping them here?
• One example query is enough in the index.md file we do not have to keep multiple examples here.
• The documentation and install sections are overlapping with the query section, I believe there are some issues with the "``" usage, please check that.
• Also please make sure that the Configuration section is in sync with the changes suggested for the config/postgres.spc file.
• We need a slight change in the way we include details of the environment variable, can you please change the section after the connection as follows: Alternatively, you can also use the standard Postgres environment variables to obtain credentials only if the other argument (connection_string) is not specified in the connection:

    export DATABASE_URL=postgres://username:password@localhost:5432/database_name

• Is the environment variable name correct?
• Under Get Involved we should include the links in - rather than * for instance

• Open source: https://github.com/jreyesr/steampipe-plugin-postgres
• Community: Join #steampipe on Slack →

  
  - `postgres`

• Please rename the config.go to connection_config.go
• plugin.go
  • The function has some extra unnecessary spaces we can restructure it as follows

    func Plugin(ctx context.Context) *plugin.Plugin {
      p := &plugin.Plugin{
          Name: "steampipe-plugin-postgres",
          DefaultTransform: transform.FromGo().NullIfZero(),
          ConnectionConfigSchema: &plugin.ConnectionConfigSchema{
              NewInstance: ConfigInstance,
              Schema:      ConfigSchema,
          },
          SchemaMode:   plugin.SchemaModeDynamic,
          TableMapFunc: PluginTables,
      }
      return p
    }

README

• We can update Install the plugin with [Steampipe](https://steampipe.io) to Download and install the latest Postgres plugin:

• Also please make sure that the Configuration section is in sync with the changes suggested for the config/postgres.spc file.

• Please include the details of authentication via environment variable using the EXPORT command

• Please include the following inder contribution section:

  
  `help wanted` issues:

• Steampipe

• Postgres Plugin

go.mod

• We should recompile the plugin with steampipe-plugin-sdk v5.6.1 and GO version 1.21

• 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 to https://steampipe.io/docs/develop/plugin-release-checklist#data-ingestion for more information

Please let us know if you have questions, happy to help 👍

[jreyesr]

Hello @madhushreeray30, and thanks for all the suggestions!

I've implemented most of them in https://github.com/jreyesr/steampipe-plugin-postgres/commit/ff39b1f20232a99e3618d89fa61197e0143788da. The only points that require a standalone comment, I think, are:

Is the environment variable name correct?

I think so. Unlike, say, the AWS CLI (where you know that you must set, for example, AWS_DEFAULT_REGION, because that's what Amazon's official CLI uses), for this plugin there's not a clear reference CLI tool. Thus, there's no One True envvar that I must match, so I used DATABASE_URL

We generally do not include the example query output in the docs, is there a particular reason for keeping them here?

Not really, I've deleted them. Thanks!

plugin.go The function has some extra unnecessary spaces we can restructure it as follows

The original spacing came from gofmt, I think it tries to align the values and thus inserts spaces. I've changed it, but my IDE fought me (it really tries to autoformat on save) :)

How are we handling the resource not found errors?

Those would return zero results, as it would happen if you ran the same query in the remote (backing) DB. This plugin doesn't call a REST API, so there are no 404 errors. The worst that can happen is that you make a query that matches no records, in which case we just return zero records (i.e. an empty table)

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.

No, there's no API behind this plugin, hence no pagination support. I can't push down LIMITs either, since that's not exposed by the Steampipe SDK.

While working with the APIs did you encounter rate limit errors?

There shouldn't be any, as the remote (backing) Postgres datastore shouldn't have rate limits in place. Postgres doesn't support those kinds of limits either.

Let me know if I've forgotten any suggestions. Again, thanks for the review!

[madhushreeray30]

Hey @jreyesr thanks for making the changes 👍. While going through the fixes here are few more suggestions that you can work on

• I believe we do not need the test.go.old file so we can remove that

• index.md

  • There is this error in the index.md can you please look into it

  • 

  • The description should end with a . > description: Steampipe plugin for proxying queries to plain Postgres databases.

  • The og_description has an extra " at the end please remove that

• postgres/*
  • Please make sure that the list calls are plural in nature for instance ListTables instead of ListTable
• go.mod
  • Please recompile the plugin with steampipe-plugin-sdk v5.6.2

[jreyesr]

All done! (see https://github.com/jreyesr/steampipe-plugin-postgres/commit/c5efa4dc9877117558d1569d5caff6a02f866693)

[madhushreeray30]

@jreyesr Thanks, the changes look good to me 👍 . Will let you know if any further changes ar required.

[jreyesr]

Hello there @madhushreeray30! Just wanted to check in to see if there are any more changes that I should make to the plugin, I've kinda forgotten to follow up on this.

Thanks for your help!