ajmaradiaga/steampipe-plugin-btp

Steampipe plugin to access SAP Business Technology Platform

steampipe-plugin-btp's Issues

Initial suggestions for plugin release. Continue on issue #1

Hey @ajmaradiaga Few things that we are missing out on this plugin are listed out, can you please work on them and then we can get it on the hub as soon as possible :

1. Can you remove the //Endpoints and //username comments as the variables are good enough to understand >

   steampipe-plugin-btp/btp/connection_config.go

   Line 13 in 316ef2c

   // Username

2. Why have we kept the required : false in access_token? Is it not a required parameter? >

   steampipe-plugin-btp/btp/connection_config.go

   Line 28 in 316ef2c

   Required: false,

3. Does this fetch a single result ? >

   steampipe-plugin-btp/btp/table_btp_accounts_global_account.go

   Line 42 in 316ef2c

   func getGlobalAccount(ctx context.Context, d *plugin.QueryData, h *plugin.HydrateData) (interface{}, error) {

   why do we have a get call in a list config?. Since you are streamlisting I believe this gives all teh global accounts, so can you please rename it to listGlobalAccounts
4. Instead of using * can you please mention the important columns>

• steampipe-plugin-btp/docs/tables/btp_accounts_directory.md

  Line 11 in 316ef2c

  *

• steampipe-plugin-btp/docs/tables/btp_entitlements_datacenter.md

  Line 11 in 316ef2c

  *

• steampipe-plugin-btp/docs/tables/btp_entitlements_datacenter.md

  Line 20 in 316ef2c

  *

• steampipe-plugin-btp/docs/tables/btp_entitlements_datacenter.md

  Line 31 in 316ef2c

  *

5. I don't think we need the connection name to be mentioned here just the table name (btp_accounts_directory) would be good as the connection name may vary depending on the number of connections someone is using >

• steampipe-plugin-btp/docs/tables/btp_accounts_directory.md

  Line 13 in 316ef2c

  btp.btp_accounts_directory

• steampipe-plugin-btp/docs/tables/btp_accounts_global_account.md

  Line 22 in 316ef2c

  btp.btp_accounts_global_account;

• steampipe-plugin-btp/docs/tables/btp_accounts_subaccount.md

  Line 85 in 316ef2c

  btp.btp_entitlements_datacenter dc

• steampipe-plugin-btp/docs/tables/btp_entitlements_datacenter.md

  Line 22 in 316ef2c

  btp.btp_entitlements_datacenter

• https://github.com/ajmaradiaga/steampipe-plugin-btp/blob/316ef2c211838a9d58ce9fcbde038b59e2c10042/docs/tables/btp_entitlements_datacenter.md?plain=1#L33C1-L33C1

• steampipe-plugin-btp/docs/tables/btp_entitlements_datacenter.md

  Line 57 in 316ef2c

  btp.btp_entitlements_datacenter dc

• steampipe-plugin-btp/docs/index.md

  Line 28 in 316ef2c

  btp.btp_accounts_global_account;

• steampipe-plugin-btp/README.md

  Line 67 in 316ef2c

  btp.btp_accounts_global_account;

6. I think we can have 1-2 more example queries for the table btp_accounts_global_account like eg : List the global accounts create din the last 7 days, List the consumption-based global accounts, List all suspended global accounts, etc.
7. What does the distinct do here is it only on the parent_guid? If so then we should put it in the second column, But why do we need it? >

   steampipe-plugin-btp/docs/tables/btp_accounts_subaccount.md

   Line 42 in 316ef2c

   select distinct

8. Please remove this extra line >

   steampipe-plugin-btp/docs/tables/btp_accounts_subaccount.md

   Line 90 in 316ef2c
9. We do not need an alias when we have only one table with no joins >

• steampipe-plugin-btp/docs/tables/btp_entitlements_assignment.md

  Line 13 in 316ef2c

  btp_entitlements_assignment bes;

• steampipe-plugin-btp/docs/tables/btp_entitlements_assignment.md

  Line 23 in 316ef2c

  btp_entitlements_assignment bes;

10. Why do we need the distinct here? Again can you please write it in the next line as distinct business_category ->> 'id' as bc_id if the distinct is needed. >

    steampipe-plugin-btp/docs/tables/btp_entitlements_assignment.md

    Line 10 in 316ef2c

    select distinct

11. Please change the WHERE to lower case >

    steampipe-plugin-btp/docs/tables/btp_entitlements_datacenter.md

    Line 23 in 316ef2c

    WHERE

12. Is the query output correct? The created_date and modified_date is not in date format >

• steampipe-plugin-btp/docs/index.md

  Line 35 in 316ef2c

  | 010788v8-7s64-1801-6680-l6g2253646b2 | My BTP global account | 1638221010619 | 1693587625761 |

• steampipe-plugin-btp/README.md

  Line 74 in 316ef2c

  | 010788v8-7s64-1801-6680-l6g2253646b2 | My BTP global account | 1638221010619 | 1693587625761 |

13. The credential section should have all 4 > Credentials, Permissions, Radius, Resolution.>

    steampipe-plugin-btp/docs/index.md

    Line 55 in 316ef2c

    | Item | Description |

14. The get involved section should contain - Community: Join #steampipe on Slack → as well >

    steampipe-plugin-btp/docs/index.md

    Line 95 in 316ef2c

    ## Get involved

15. The install command is incorrect it should be > steampipe plugin install ajmaradiaga/btp

• steampipe-plugin-btp/docs/index.md

  Line 50 in 316ef2c

  steampipe plugin install btp

• steampipe-plugin-btp/README.md

  Line 16 in 316ef2c

  steampipe plugin install btp

16. The community link is missing after documentation - Community: Slack Channel > https://github.com/ajmaradiaga/steampipe-plugin-btp/blob/316ef2c211838a9d58ce9fcbde038b59e2c10042/README.md?plain=1#L6C1-L6C1

Please reach out to us in case of any don't, thanks!

Initial suggestions for plugin release

Thanks @ajmaradiaga 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/btp.spc

• We should have a single file named btp.spc under the config folder that will contain a basic connection for the plugin (see namecheap plugin for reference)
• Could you please update the btp.spc file to follow the format of the namecheap plugin
• Can you mention which of these parameters are required and which are optional as done in the namecheap plugin
• We should mention values for all the config arguments however commented out and the values should be in the format of the actual values (however incorrect)
• We should also add details to set these parameters via the environment variable (if any)

docs/tables/*

• We should have a folder called docs under which we will have a single file called index.md (reference) and individual table docs for each table
• The table docs should be named as eg > btp_datacenter.md (note: The table names should be singular)
• Each table doc should have the following parts >
  • A heading in the format > # Table: btp_datacenter
  • A description of the table
  • A number of queries which will have the title followed by the query (Note: We do not include the query outputs in the docs)

docs/index.md

• Needs to be added (reference)

btp/*

• Please add the configuration for setting the config arguments via the environment variable
• The table file names should be in singular and in snake_case for example : table_btp_accounts_directories.go
• The table file name should be in sync with the table name in the code
• It is better to print the type of error when we encounter any like connection_error or api_error for easier debugging if needed

btp/table_btp_datacenter

• The name of the file should be table_btp_datacenter.go
• The function name should be tableBTPDatacenter
• The name of the table should be btp_datacenter
• The column descriptions should end with a .
• The geo_access column does not have a description we should add it
• It is better to use d.EqualsQualString["region"] instead of equalQuals["region"].GetStringValue()

btp/table_btp_directory

• The name of the file should be table_btp_directory.go
• The function name should be tableBTPDirectory
• The name of the table should be btp_directory
• The column descriptions should end with a .
• The directory_type and parent_guid1 columns do not have a description we should add it
• It is better to use d.EqualsQualString["region"] instead of equalQuals["region"].GetStringValue()

btp/table_btp_global_account

• The name of the file should be table_btp_global_account.go
• The name of the table should be btp_global_account
• The name of the list call should be listGlobalAccounts
• The column descriptions should end with a .

btp/table_btp_subaccount

• The name of the file should be table_btp_subaccount.go
• The name of the table should be btp_subaccount
• The name of the list call should be listSubaccounts
• The column descriptions should end with a .
• The parent_type column does not have a description we should add it

btp/plugin.go

• The list of tables should be sorted in ascending order

btp/utils.go

• Please rename the btp.go file to utils.go

bt/types.go

• Rename to types.go

README.md

• Please update the file to follow the format of the namecheap plugin
• Please include all the necessary sections as mentioned above

CHANGELOG.md

• Please draft an initial Changelog for the plugin release, an example would be as follows:

## v0.0.1 [TBD]

_What's new?_

- New tables added
  - [btp_datacenter](https://github.com/ajmaradiaga/steampipe-plugin-btp/blob/main/btp/table_btp_datacenter.go)
  - [btp_directory](https://github.com/ajmaradiaga/steampipe-plugin-btp/blob/main/btp/table_btp_directory.go)
  - [btp_global_account](https://github.com/ajmaradiaga/steampipe-plugin-btp/blob/main/btp/table_btp_global_account.go)
  - [btp_subaccount](https://github.com/ajmaradiaga/steampipe-plugin-btp/blob/main/btp/table_btp_subaccount.go)

Please note that the example contains the name in singular assuming that the changelog will be added after updating the table names.

Makefile

• The path is incorrect please update it to

go build -o ~/.steampipe/plugins/hub.steampipe.io/plugins/ajmaradiaga/btp@latest/teampipe-plugin-btp.plugin *.go

Follow-up questions

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