Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Check update have required parameter. #26

Open
partamonov opened this issue May 3, 2018 · 4 comments
Open

Check update have required parameter. #26

partamonov opened this issue May 3, 2018 · 4 comments

Comments

@partamonov
Copy link
Contributor

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
Copy link
Contributor Author

partamonov commented May 3, 2018

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)

@russellcardullo
Copy link
Owner

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.

RafPe added a commit to RafPe/go-pingdom that referenced this issue May 14, 2018
@RafPe
Copy link

RafPe commented May 14, 2018

@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.

@partamonov
Copy link
Contributor Author

@russellcardullo could you please review?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants