Added "SHOW RESOURCE MONITORS" #8

paulsewardasc · 2022-05-12T09:17:42Z

Example query results

Results

select
  name,
  credit_quota,
  used_credits,
  remaining_credits
from
  snowflake_resource_monitor;
+--------------------------------+--------------+--------------+-------------------+
| name                           | credit_quota | used_credits | remaining_credits |
+--------------------------------+--------------+--------------+-------------------+
| WAREHOUSE SEARCH               | 10           | 0            | 10                |
| TEST_LARGE                     | 5            | 0            | 5                 |
| ABC_MEDIUM                     | 25           | 0.57         | 24.43             |
+--------------------------------+--------------+--------------+-------------------+


select account,
    name as warehouse,
    credit_quota,
    used_credits,
    remaining_credits,
    round(cast(used_credits/credit_quota*100 as numeric), 1) as "% Used",
    case
        when used_credits/credit_quota*100 > 90 then 'alert'
        when used_credits/credit_quota*100 > 75 then 'warning'
        else 'ok'
    end as type
from snowflake_resource_monitor
where used_credits/credit_quota*100 > 75
order by used_credits/credit_quota desc
+--------------+----------------------------+--------------+--------------+-------------------+--------+---------+
| account      | warehouse                  | credit_quota | used_credits | remaining_credits | % Used | type    |
+--------------+----------------------------+--------------+--------------+-------------------+--------+---------+
| acount1      | WAREHOUSE1                 | 7            | 8.54         | -1.54             | 122.0  | alert   |
| acount1      | WAREHOUSE2                 | 13           | 15.62        | -2.62             | 120.2  | alert   |
| acount1      | WAREHOUSE3                 | 300          | 270.58       | 29.42             | 90.2   | alert   |
| acount1      | WAREHOUSE TEST             | 578          | 451.98       | 126.02            | 78.2   | warning |
+--------------+----------------------------+--------------+--------------+-------------------+--------+---------+

select
  name,
  notify_at, 
  suspend_at, 
  suspend_immediately_at
from
  snowflake_resource_monitor
+-----------------+------------------+------------+------------------------+
| name            | notify_at        | suspend_at | suspend_immediately_at |
+-----------------+------------------+------------+------------------------+
| WAREHOUSE1121   | 100%,50%,75%,90% | <null>     | <null>                 |
| WAREHOUSE TEST  | 25%,50%,75%      | <null>     | <null>                 |
| DATA WH001      | 100%,50%         | <null>     | <null>                 |
| SEARCH          | 100%,25%,75%,90% | 90%        | 100%                   |
+-----------------+------------------+------------+------------------------+

rajlearner17 · 2022-05-12T10:01:47Z

@paulsewardasc Awesome to see this PR. Thank you 👍
We will review it and get back to you as needed.

cbruno10

Hey @paulsewardasc , thanks for working on adding this table and opening a PR!

I've added some comments and suggestions, can you please have a look at them when you get a chance? If you have any questions, please let me know!

cbruno10 · 2022-05-12T12:50:26Z

README.md

@@ -31,7 +31,32 @@ where
  (last_success_login > now() - interval '30 days')
  and last_success_login is not null;
 ```
+## Examples


Instead of adding examples here, can you please create a new table document, e.g., https://github.com/turbot/steampipe-plugin-snowflake/blob/main/docs/tables/snowflake_database.md? These docs are helpful as they show up on https://hub.steampipe.io/plugins/turbot/snowflake/tables.

Also, for the examples, can you please follow our standard formatting as per Table Documentation Standards?

@paulsewardasc Did you push up the new commit yet? I don't see it in the Commits list yet.

It is here, do I need to do something else? It may not have been here last night.

https://github.com/paulsewardasc/steampipe-plugin-snowflake/tree/resource_monitors

cbruno10 · 2022-05-12T12:50:42Z

snowflake/table_snowflake_resource_monitor.go

+	return nil, nil
+}
+
+func ScanResourceMonitor(row *sqlx.Row) (*ResourceMonitor, error) {


Is this function used anywhere? If not, can you please remove it?

cbruno10 · 2022-05-12T12:51:40Z

snowflake/table_snowflake_resource_monitor.go

+
+type ResourceMonitor struct {
+	Name              sql.NullString `json:"name" db:"name"`
+  CreditQuota       sql.NullString `json:"credit_quota" db:"credit_quota"`


Some of the formatting looks a bit off here, can you please run gofmt -w <filename> to update the file to use the standard Go formatting?

cbruno10 · 2022-05-12T12:52:59Z

snowflake/table_snowflake_resource_monitor.go

+      {Name: "frequency", Type: proto.ColumnType_STRING, Description: "Daily, Weekly, etc"},
+      {Name: "start_time", Type: proto.ColumnType_TIMESTAMP, Description: "Date and time when the monitor was started."},
+      {Name: "end_time", Type: proto.ColumnType_TIMESTAMP, Description: "Date and time when the monitor was stopped."},
+      {Name: "notify_at", Type: proto.ColumnType_STRING, Description: "Levels to which to alert"},


Suggested change

{Name: "notify_at", Type: proto.ColumnType_STRING, Description: "Levels to which to alert"},

{Name: "notify_at", Type: proto.ColumnType_STRING, Description: "Levels to which to alert."},

Please see Table and Column Descriptions for more information on formatting descriptions.

cbruno10 · 2022-05-12T12:53:29Z

snowflake/table_snowflake_resource_monitor.go

+
+func tableSnowflakeResourceMonitor(_ context.Context) *plugin.Table {
+	return &plugin.Table{
+		Name:        "snowflake_resource_monitors",


Suggested change

Name: "snowflake_resource_monitors",

Name: "snowflake_resource_monitor",

Can you please make the table name singular?

Done although the command run is "SHOW RESOURCE MONITORS"

snowflake/table_snowflake_resource_monitor.go

cbruno10 · 2022-05-12T12:56:19Z

snowflake/table_snowflake_resource_monitor.go

+  EndTime           sql.NullTime `json:"end_time" db:"end_time"`
+  NotifyAt          sql.NullString `json:"notify_at" db:"notify_at"`
+  SuspendAt         sql.NullString `json:"suspend_at" db:"suspend_at"`
+  SuspendImdAt      sql.NullString `json:"suspend_immediately_at" db:"suspend_immediately_at"`


Suggested change

SuspendImdAt sql.NullString `json:"suspend_immediately_at" db:"suspend_immediately_at"`

SuspendImmediatelyAt sql.NullString `json:"suspend_immediately_at" db:"suspend_immediately_at"`

We prefer to use full names (even if they're a bit long) so there's no ambiguity and all names are consistent

cbruno10 · 2022-05-12T12:57:31Z

snowflake/table_snowflake_resource_monitor.go

+	logger := plugin.Logger(ctx)
+	db, err := connect(ctx, d)
+	if err != nil {
+		logger.Error("snowflake_warehouse.listSnowflakeResourceMonitors", "connnection.error", err)


Suggested change

logger.Error("snowflake_warehouse.listSnowflakeResourceMonitors", "connnection.error", err)

logger.Error("snowflake_resource_monitor.listSnowflakeResourceMonitors", "connnection.error", err)

Can you please fix other logging statements like this as well?

cbruno10 · 2022-05-12T13:03:03Z

snowflake/table_snowflake_resource_monitor.go

+	}
+	defer rows.Close()
+
+	dbs := []ResourceMonitor{}


Suggested change

dbs := []ResourceMonitor{}

resourceMonitors := []ResourceMonitor{}

We should match the variable name more closely with what we're retrieving

cbruno10

@paulsewardasc Thanks for making the updates quickly! I've added a few more suggestions/comments, but I believe these should be the latest round of suggestions.

Can you please have a look when you get a chance? If you have any questions, please let me know.

snowflake/plugin.go

snowflake/table_snowflake_resource_monitor.go

cbruno10 · 2022-05-13T18:31:01Z

snowflake/table_snowflake_resource_monitor.go

+			{Name: "end_time", Type: proto.ColumnType_TIMESTAMP, Description: "Date and time when the monitor was stopped."},
+			{Name: "notify_at", Type: proto.ColumnType_STRING, Description: "Levels to which to alert."},
+			{Name: "suspend_at", Type: proto.ColumnType_STRING, Description: "Levels to which to suspend warehouse"},
+			{Name: "suspend_immediately_at", Type: proto.ColumnType_STRING, Description: "Levels to which to suspend warehouse"},


Does this column's value ever have more than 1 level at which the warehouse will be suspended? If so, is it a comma delimited list (or a list with another delimiter)? Going off of the description, it seems like we could expect more than 1 level.

Do you have a link to where you pulled this description from? That may be helpful for me to compare the current descriptions vs. what the docs/other sources mention.

This column can have 1 or more values, having problems compiling the code at the moment, the function keeps show the show warehouse schema

@paulsewardasc Are you still having troubles compiling the plugin? Please let me know here or over Slack if there's anything I can help with.

Fixed it, the plugin.go file was wrong!

The credit_quota is defined in snowflake as a variant data type.

The remaining_credits is defined as a float and suspend is a NUMBER(38,0) is that helps?

READER_ACCOUNT_NAME VARCHAR(16777216)
NAME VARCHAR(16777216)
CREATED TIMESTAMP_LTZ(6)
CREDIT_QUOTA VARIANT
USED_CREDITS VARIANT
REMAINING_CREDITS FLOAT
OWNER VARCHAR(16777216)
WAREHOUSES VARCHAR(16777216)
NOTIFY NUMBER(38,0)
SUSPEND NUMBER(38,0)
SUSPEND_IMMEDIATE NUMBER(38,0)
LEVEL VARCHAR(9)
READER_ACCOUNT_DELETED_ON TIMESTAMP_LTZ(6)

For the credit_quota column, I think OK to leave as a double. In the example query results you added, they appear to be integers instead of doubles, but if there's a possibility there's precision as per https://docs.snowflake.com/en/sql-reference/data-types-numeric.html#number, then double is correct.

For notfiy_at, suspend_at, and suspend_at_immediately, do you have example query results that include single value and multi-value rows? I'd be curious to see how the data looks since the column types are currently marked as STRING, but if there can be multiple values, should the column types be JSON instead (which handles arrays)?

docs/tables/snowflake_resource_monitor.md

cbruno10 · 2022-05-19T13:09:39Z

Hey @paulsewardasc , thanks for the quick updates a few days ago! I believe the last set of questions I have before I'm ready to approve is mentioned in #8 (comment), when you get a chance can you please have a look? If there's anything I can expand on, please let me know. Thanks!

paulsewardasc · 2022-05-19T14:25:25Z

Hey @paulsewardasc , thanks for the quick updates a few days ago! I believe the last set of questions I have before I'm ready to approve is mentioned in #8 (comment), when you get a chance can you please have a look? If there's anything I can expand on, please let me know. Thanks!

Hi, I am not sure what I need to do, I have added an example.

cbruno10 · 2022-05-19T14:52:03Z

@paulsewardasc Sorry about that, I didn't realize you updated the PR comment/description with a new example showing those columns, I was originally looking in the review comment chain where I asked for the examples.

Based on the examples, the column type STRING makes sense.

cbruno10 · 2022-05-19T14:54:04Z

@paulsewardasc Thanks for creating the PR and working through the various reviews! We really appreciate the contribution and plan to release a new version of this plugin this week.

paulsewardasc added 3 commits May 12, 2022 10:11

Added SHOW RESOURCE MONITORS

3024140

Added example

ef7a5cf

Added another example

cac614a

rajlearner17 requested a review from LalitLab May 12, 2022 09:59

cbruno10 requested changes May 12, 2022

View reviewed changes

paulsewardasc added 4 commits May 12, 2022 17:03

Made documentation changes

6134777

Changes made to fit standards

0d86807

logger changes

d110306

Removal of warehouse references

3ea1f76

cbruno10 requested changes May 13, 2022

View reviewed changes

paulsewardasc added 2 commits May 16, 2022 12:14

Changes made to match formating

9789aab

Fixed wrong reference to function

60eb238

cbruno10 approved these changes May 19, 2022

View reviewed changes

cbruno10 merged commit cf12688 into turbot:main May 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added "SHOW RESOURCE MONITORS" #8

Added "SHOW RESOURCE MONITORS" #8

paulsewardasc commented May 12, 2022 •

edited

Loading

rajlearner17 commented May 12, 2022

cbruno10 left a comment

cbruno10 May 12, 2022

paulsewardasc May 12, 2022

cbruno10 May 12, 2022

paulsewardasc May 13, 2022

cbruno10 May 12, 2022

paulsewardasc May 12, 2022

cbruno10 May 12, 2022

paulsewardasc May 12, 2022

cbruno10 May 12, 2022

paulsewardasc May 12, 2022

cbruno10 May 12, 2022

paulsewardasc May 12, 2022

cbruno10 May 12, 2022

paulsewardasc May 12, 2022

cbruno10 May 12, 2022

paulsewardasc May 12, 2022

cbruno10 May 12, 2022

paulsewardasc May 12, 2022

cbruno10 left a comment

cbruno10 May 13, 2022

paulsewardasc May 16, 2022

cbruno10 May 16, 2022

paulsewardasc May 16, 2022

paulsewardasc May 16, 2022

paulsewardasc May 16, 2022

paulsewardasc May 16, 2022

cbruno10 May 16, 2022

cbruno10 commented May 19, 2022

paulsewardasc commented May 19, 2022

cbruno10 commented May 19, 2022

cbruno10 commented May 19, 2022

	{Name: "notify_at", Type: proto.ColumnType_STRING, Description: "Levels to which to alert"},
	{Name: "notify_at", Type: proto.ColumnType_STRING, Description: "Levels to which to alert."},

	Name: "snowflake_resource_monitors",
	Name: "snowflake_resource_monitor",

	SuspendImdAt sql.NullString `json:"suspend_immediately_at" db:"suspend_immediately_at"`
	SuspendImmediatelyAt sql.NullString `json:"suspend_immediately_at" db:"suspend_immediately_at"`

	logger.Error("snowflake_warehouse.listSnowflakeResourceMonitors", "connnection.error", err)
	logger.Error("snowflake_resource_monitor.listSnowflakeResourceMonitors", "connnection.error", err)

	dbs := []ResourceMonitor{}
	resourceMonitors := []ResourceMonitor{}

Added "SHOW RESOURCE MONITORS" #8

Added "SHOW RESOURCE MONITORS" #8

Conversation

paulsewardasc commented May 12, 2022 • edited Loading

Example query results

rajlearner17 commented May 12, 2022

cbruno10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbruno10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbruno10 commented May 19, 2022

paulsewardasc commented May 19, 2022

cbruno10 commented May 19, 2022

cbruno10 commented May 19, 2022

paulsewardasc commented May 12, 2022 •

edited

Loading