salt-extensions/saltext-pushover

Salt Extension for interacting with Pushover

saltext-pushover's Issues

[BUG] `device`, `retry`, `priority` and `expire` are required, but should not be

Description
When device, retry, priority or expire is not specified, the API returns an error since this extension tries to send None values. I'm not sure when the behavior changed or if this ever worked as expected, but I have carried a custom patch for some time now.

I'll quickly fix some basic issues like this, but really think this extension needs a proper rewrite in a v2.

Steps to Reproduce the behavior
Try to send a message without specifying one of the values.

Expected behavior
Works.

Screenshots
Crash.

[TECH DEBT] Required values should be required

Description of the tech debt to be addressed, include links and screenshots

Functions in this extension declare default values for all parameters, even if they are required. This results in a suboptimal interface and code layout (which needs to check for non-falsy required values).

Example:

message is a required parameter for pushover.post_message. It's the third positional argument though, after user and device, both of which are read from the configuration if unspecified. Thus one cannot simply call salt["pushover.notify"]("My message").

[BUG] Superfluous parameters

Description
Several functions in this extension take an api_version parameter, which is completely ignored.
The (superfluous) state module exposes a sound parameter, which is ignored.
The query utils func exposes token, which is ignored.

Steps to Reproduce the behavior
Set api_version to anything and see the extension still working.

Expected behavior
Don't expose parameters which do nothing.

There is only a single API version. We don't know how a successor will be structured, so we cannot assume the module code to just work. Only the query util func should retain the parameter for custom modules.

[BUG] `pushover.post_message` state ignores `user` from the configuration

Description
See title.

Sorry for keeping this brief, but this is a basic functionality just not working, so not much to reproduce.

saltext-pushover/src/saltext/pushover/states/pushover.py

Lines 109 to 111 in 8b3152f

	if not user:
	ret["comment"] = f"PushOver user is missing: {user}"
	return ret

[BUG] `sound` and `user` are validated during each request

Description
See title. This causes 2 (3 if sound is specified) API calls for each notification.

Steps to Reproduce the behavior
Use the extension.

Expected behavior
If the validation works, cache it and only revalidate infrequently.