go-pingdom icon indicating copy to clipboard operation
go-pingdom copied to clipboard

Check update have required parameter.

Open partamonov opened this issue 6 years ago • 4 comments

Check update have required parameter. Based on Pingdom documentation there is no mandatory parameters.

Now pingdom.Checks.Update(id, check) fails with errors from Valid() like

2018/05/03 10:15:44 Invalid value for `Name`.  Must contain non-empty string

partamonov avatar May 03 '18 08:05 partamonov

Remove check.Valid from Update and all PutParams() should be added only if they are specified.

Or update readme should looks like:

        check, _ := client.Checks.Read(id)

	modifyCheck := pingdom.HttpCheck{
		Name:             check.Name,
		Hostname:         check.Hostname,
		Resolution:       check.Resolution,
		Encryption:       check.Type.HTTP.Encryption,
		Paused:           check.Paused,
		NotifyAgainEvery: check.NotifyAgainEvery,
		NotifyWhenBackup: check.NotifyWhenBackup,
                ... // All params from Read should be passed in
	}

	updatedCheck, checkerr := client.Checks.Update(id, &modifyCheck)

partamonov avatar May 03 '18 09:05 partamonov

I think fixing the docs would be best short term.

The challenge here is being able to determine which values are missing in the check to filter the PutParams. Go is missing an option type, so we'd have to do something like

type HttpCheck struct{
    Name       *string
    Hostname   *string
    Resolution *int
    ...
}

But that makes it more cumbersome to use the types.

Extending PutParams to filter empty args or 0 valued ints might work for most use cases, but it wouldn't address cases like if someone wanted to explicitly set a value to empty they wouldn't be able to if we filtered those on update.

For now we should definitely update the README. If I get a chance in the next few weeks I might try a POC of what moving the types to something where all fields are optional would look like. I think if we go down that path we'll likely want to do so for the other types as well.

russellcardullo avatar May 04 '18 21:05 russellcardullo

@russellcardullo Hi - so based on what we have seen around in order to properly modify we need to send out only items we are interested in. Hence for short term I propose using method which will get our PutParams and clean it out from null values. and if we need to null some values we can always use existing one.

RafPe avatar May 14 '18 15:05 RafPe

@russellcardullo could you please review?

partamonov avatar May 22 '18 15:05 partamonov