longportapp / steampipe-plugin-longport Goto Github PK

Thanks @huacnlee 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/longport.spc

• Could you please update the longport.spc file to follow the format of the namecheap plugin.

• The plugin name in the connection should be plugin = "longportapp/longport".

• Can you please give a brief description of the config parameter that tells which of them are required and which of them are optional, and then set the parameter with a demo value that looks similar to the actual value?

• Also add the details of the environment variables that we can use instead of the config parameters.

• Both the descriptions and the config parameters should be commented out and the values should be in the format of the actual values (however incorrect).
  For instance:

connection "longport" {
  plugin = "longportapp/longport"

  # The longport app key. Required.
  # This can also be set via the `LONGPORT_APP_KEY` environment variable.
  # app_key      = "a12b34cd-56ef-78gh-9i00-jk123lmn456o"

  # The longport app secret. Required.
  # This can also be set via the `LONGPORT_APP_SECRET` environment variable.
  # app_secret   = "sEcReT-9876-AbCd-5432-EfGhIjKlMnOp"

  # The longport access token. Required.
  # This can also be set via the `LONGPORT_ACCESS_TOKEN` environment variable.
  # access_token = "token_1234abcd5678efgh9012ijkl"
}

(The values that I have provided are truly demo values can you replace them with values that look like the original ones although changing parts of them to maintain privacy)

• docs/LICENSE

• Please add the doc license to the docs folder.

• docs/tables/*

• Please make sure the table names are singular, for instance, longport_broker instead of longport_brokers.

• We do not generally keep the output of the example queries. Is there a specific reason for doing that?

• We have slightly changed the pattern of the table docs can you please take this reference and change it to this format. It should include queries in both Postgres and SQLite.

• Please update the format of the example queries using https://www.freeformatter.com/sql-formatter.html#before-output (2 indent spaces).

• It's better to avoid the use of select * instead we can just query the important columns.

• The table docs look very light, can you please add 2-3 more example queries to each table.

• docs/index.md

• Could you please update the index.md file to follow the format of the namecheap plugin.

• Please add engines: ["steampipe", "sqlite", "postgres", "export"] in the top section

• Update the steampipe definition to [Steampipe](https://steampipe.io) is an open-source zero-ETL engine to instantly query cloud APIs using SQL.

• Change the Get Started to Quick Start and add the document section before it.

## Documentation

- **[Table definitions & examples →](/plugins/longportapp/longport/tables)**

• Please update Download and install the latest Twitter plugin: to use Longport instead of Twitter.
• Please update the credential section to include all 4 items (reference).
• Please add Configure your account details in ~/.steampipe/config/longport.spc: in the configuration section.
• Please make sure the configuration section is in sync with the updated longport.spc file.
• Please remove the Get Involved section and add the details to set these parameters via the environment variables.

Alternatively, you can also use the standard Longport environment variables to obtain credentials only if other arguments (app_key, app_secret, and access_token) are not specified in the connection:

export LONGPORT_APP_KEY=a12b34cd-56ef-78gh-9i00-jk123lmn456o
export LONGPORT_APP_SECRET=sEcReT-9876-AbCd-5432-EfGhIjKlMnOp
export LONGPORT_ACCESS_TOKEN=token_1234abcd5678efgh9012ijkl

README.md

• Could you please update the file to follow the format of the namecheap plugin.

• The basic sections are missing so it would be better if you could follow the format from the above reference and then we can further see if any changes are required.

• longport/columns.go

• We generally keep the columns in the respective table codes and if there are common columns used across we add it to a common_collumns.go file. Could you please shift the columns to the respective tables and if any common column is used keep it here after changing the file name to common_collumns.go.

• Each column description should end with a .

• Do we need to add the transform function in all the columns? The default conversation is from camel case to snake case so I think we do not need the transform when we are keeping the column names same just changing from camel case to snake case. Eg > TradeStatus would be changed by GO to trade_status without any transform function.

• longport/connection_config.go

• Please update the file name from config.go to connection_config.go

• Update the cty to hcl and remove theConfigSchema (as per our latest changes across all plugins)

• longport/longport_*.go

• Please give the table description in the form of a complete sentence. For some tables, it is a bit messy.

• Please update the file name to use singular naming for example longport_broker.go

• Please update the function name to the format tableLongportBroker. In some tables, we have followed the longport_history_executions format which should also be updated to tableLongportHistoryExecutions. Apply to all tables.

• Whenever we get an error we should print the type of error in the logger for example plugin.Logger(ctx).Error("longport_broker.listBrokers", "connection_error", err) when the error is during client creation and plugin.Logger(ctx).Error("longport_broker.listBrokers", "api_error", err) when the error is during the API call. Please apply this to all tables. (reference)

• longport/plugin.go

• The table names should be sorted in ascending order. (Make sure to use the updated function names).

• We no longer need the ConfigSchema so we can remove it.

• .github

• The folder does not have all the required files can you please update it taking reference from here.

• DEVELOPMENT.md and version.json

• We do not require these files as the information in README and index docs are sufficient.

• go.mod

• Please update the steampipe-plugin-sdk to v5.8.0

• CHANGELOG.md

• Please draft an initial CHANGELOG that will include the following

## v0.0.1 [TBD]

_What's new?_

- New tables added
  - [longport_broker](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_broker)
  - [longport_candlestick](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_candlesticks)
  - [longport_depth](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_depth)
  - [longport_history_executions](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_history_executions)
  - [longport_history_orders](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_history_orders)
  - [longport_intraday](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_intraday)
  - [longport_option_quote](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_option_quote)
  - [longport_participant](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_participant)
  - [longport_quote](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_quote)
  - [longport_static_info](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_static_info)
  - [longport_today_execution](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_today_execution)
  - [longport_today_order](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_today_order)
  - [longport_trade](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_trade)
  - [longport_warrant_quote](https://hub.steampipe.io/plugins/longportapp/longport/tables/longport_warrant_quote)

• LICENSE

• Please update the file similar to this.

• Makefile

• Please update the make command to

install:
	go build -o ~/.steampipe/plugins/hub.steampipe.io/plugins/longportapp/longport@latest/steampipe-plugin-longport.plugin *.go

• #3

• 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 👍.