-
Notifications
You must be signed in to change notification settings - Fork 386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add empty_result_state
attribute to the databricks_sql_alert
resource
#2724
Conversation
// This is a workaround for Go SDK problem, will be fixed there. | ||
var err error | ||
if a.Options.EmptyResultState != "" { | ||
err = ca.Options.EmptyResultState.Set(a.Options.EmptyResultState) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should throw an error if the value is not supported
Currently supported values are
unknown
,triggered
,ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check should be done on the backend. The main problem right now is that I can't simply do ca.Options.EmptyResultState.Set(a.Options.EmptyResultState)
if the value is omitted - SDK returns an error.
@@ -46,14 +47,18 @@ func (a *AlertEntity) toCreateAlertApiObject(s map[string]*schema.Schema, data * | |||
Op: a.Options.Op, | |||
Value: a.Options.Value, | |||
} | |||
|
|||
return ca, nil | |||
// This is a workaround for Go SDK problem, will be fixed there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which problem is this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with SDK that it doesn't accept empty string for optional enums
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tracked here: databricks/databricks-sdk-go#630
Codecov Report
@@ Coverage Diff @@
## master #2724 +/- ##
==========================================
- Coverage 84.82% 84.78% -0.04%
==========================================
Files 148 148
Lines 12991 12998 +7
==========================================
+ Hits 11019 11020 +1
- Misses 1378 1384 +6
Partials 594 594
|
@alexott I noticed the documentation for this change is incorrect -- the new |
@junwei-db it's just a doc problem, implementation is in the correct place |
Right, that's what I was referring to. Could you help fix the doc? |
See #2834 |
Changes
This fixes #2662
Tests
make test
run locallydocs/
folderinternal/acceptance