Skip to content
This repository has been archived by the owner on Jun 2, 2024. It is now read-only.

Feature/77 sli configuration #81

Merged
merged 11 commits into from
Dec 20, 2020
Merged

Conversation

jonassiefker
Copy link
Collaborator

No description provided.

Copy link
Owner

@gessnerfl gessnerfl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SliConfigFieldMetricConfiguration: SliConfigMetricConfiguration,
SliConfigFieldSliEntity: SliConfigSliEntity,
},
SchemaVersion: 1,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initial schema version should be 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

misconception on my part, initial schema version set to 0

SliConfigFieldSliEntity: SliConfigSliEntity,
},
SchemaVersion: 1,
StateUpgraders: []schema.StateUpgrader{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the initial version no state upgrader is required

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

misconception on my part, state upgrader removed

func mapStateToDataObjectForSliConfig(d *schema.ResourceData, formatter utils.ResourceNameFormatter) (restapi.InstanaDataObject, error) {
metricConfigurationsList := d.Get(SliConfigFieldMetricConfiguration).([]interface{})
var metricConfiguration restapi.MetricConfiguration
if len(metricConfigurationsList) > 0 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be moved into a separate method mapMetricConfigurationEntityFromState()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored into separate method


sliEntitiesList := d.Get(SliConfigFieldSliEntity).([]interface{})
var sliEntity restapi.SliEntity
if len(sliEntitiesList) > 0 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be moved into a separate method mapSliEntityListFromState()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored into separate method

ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) {
v := val.(float64)
if v <= 0 {
errs = append(errs, fmt.Errorf("Metric threshold must be higher than 0"))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grammar: "must be greater than"
test case missing for the validation

instana/restapi/sli-config-api.go Show resolved Hide resolved
instana/restapi/sli-config-api.go Show resolved Hide resolved
instana/restapi/sli-config-api.go Show resolved Hide resolved
Copy link
Owner

@gessnerfl gessnerfl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation of the resource is missing in docs/resources

return d.Get(SliConfigFieldFullName).(string)
}

func sliConfigSchemaV0() *schema.Resource {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not needed anymore

@@ -56,3 +57,7 @@ func (api *baseInstanaAPI) AlertingChannels() RestResource {
func (api *baseInstanaAPI) AlertingConfigurations() RestResource {
return NewRestResource(AlertsResourcePath, NewAlertingConfigurationUnmarshaller(), api.client)
}

func (api *baseInstanaAPI) SliConfigs() RestResource {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test missing

@gessnerfl gessnerfl merged commit 891a43b into master Dec 20, 2020
@gessnerfl gessnerfl deleted the feature/77-sli-configuration branch December 20, 2020 19:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants