Replace incorrect usage of term "receivers" to "integrations" #73
Conversation
| Err: fmt.Errorf("failed to parse notifier %s (UID: %s): %w", receiver.Name, receiver.UID, err), | ||
| return GrafanaReceiverConfig{}, &IntegrationValidationError{ | ||
| Integration: receiver, | ||
| Err: err, |
There was a problem hiding this comment.
Remove formatting because the error is formatted upstream (see https://github.com/grafana/alerting/pull/73/files#diff-4e5bf7fedd50378f77d70c59a65075ff01da019e19113d593edc0df932ed8c15R526-R531)
| type GrafanaReceivers struct { | ||
| Receivers []*GrafanaReceiver `yaml:"grafana_managed_receiver_configs,omitempty" json:"grafana_managed_receiver_configs,omitempty"` | ||
| type GrafanaIntegrations struct { | ||
| Integrations []*GrafanaIntegrationConfig `yaml:"grafana_managed_receiver_configs,omitempty" json:"grafana_managed_receiver_configs,omitempty"` |
There was a problem hiding this comment.
Do we have plans to migrate the Yaml/JSON? Seems like a pain doesn't it?
There was a problem hiding this comment.
I think we will migrate to the structure upstream Alertmanager offers (a dictionary of lists).
| func TestReceiverTimeoutError_Error(t *testing.T) { | ||
| e := ReceiverTimeoutError{ | ||
| Receiver: &GrafanaReceiver{ | ||
| e := IntegrationTimeoutError{ |
There was a problem hiding this comment.
Shouldn't this be something like
| e := IntegrationTimeoutError{ | |
| e := InvalidIntegrationError{ |
There was a problem hiding this comment.
No sure I understand. Do you suggest renaming ReceiverTimeoutError to InvalidIntegrationError?
There was a problem hiding this comment.
I'm not sure what happened there, when I commented GitHub showed something else.
| @@ -68,19 +57,19 @@ func (e timeoutError) Timeout() bool { | |||
| } | |||
|
|
|||
| func TestProcessNotifierError(t *testing.T) { | |||
| t.Run("assert ReceiverTimeoutError is returned for context deadline exceeded", func(t *testing.T) { | |||
| r := &GrafanaReceiver{ | |||
| t.Run("assert IntegrationTimeoutError is returned for context deadline exceeded", func(t *testing.T) { | |||
There was a problem hiding this comment.
I'm a bit confused, is there some behavioural change here too?
There was a problem hiding this comment.
Not sure I understand what you mean. I do not see any changes in this test other than names
There was a problem hiding this comment.
Again this now looks different from when I commented, GitHub being weird?
|
Approved Yuri, what are the next steps needed to finish this renaming in Grafana? |
|
The next steps would be to update the module version in Grafana update usages and do renaming there if necessary. |
…actoring (#66622) * update to alerting 20230418161049-5f374e58cb32 * rename renamed structs in grafana/alerting#73 * update ValidateContactPoint to use BuildReceiverConfiguration * update logger factory according to changes * rewrite integration builder Co-authored-by: Santiago <santiagohernandez.1997@gmail.com>
The term "receivers" is used in many places and is confused with integrations. This PR renames structs, functions and log\error messages to use the correct term for the context.
This PR renames:
Also, it removes struct InvalidReceiverError because it is not used anywhere and can be replaced by IntegrationValidationError in usages.
This PR is structured the way so it is simpler to review it by commit.