Add option for detached HUP on start_os_update

[jaomaloy]

This will call the v2 actions endpoint for resinhup which runs a detached version of HUP that increases HUP reliability on slow networks but will offer
no status updates such as in_progress.

Change-type: minor

[github-advanced-security]

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

[kb2ma]

First pass review. Have not tested operation yet.

[otaviojacobi]

Suggested change

	action_api_version = "v2" if run_detached is True else self.__settings.get("device_actions_endpoint_version")
	action_api_version = "v2" if run_detached else self.__settings.get("device_actions_endpoint_version")

[otaviojacobi]

@kb2ma @jaomaloy Are we using v2 actions endpoint only on for HuP or should the entire SDK move to the new actions endpoint?

[jaomaloy]

v2 is only for detached HUP at this point @otaviojacobi

[kb2ma]

Good catch on the "is True", @otaviojacobi . If the user happens to use runDetached="True", that still works with your change since the test now is for Truthiness. I'm a little surprised use of a string isn't type checked though since it's typed as Optional[bool] .

[otaviojacobi]

Yes, the key thing here is to (similar to typescript) always think of the type system as best effort. There is only so much you can do on a dynamically typed languages (js/python) to avoid these mistakes. There is the argument of if someone does run_detatched="False" it will still be truthy, however considering it defaults to False I think someone messing with this parameter probably wants to run detatched

[otaviojacobi]

We can also always validate that it is either True False or None and throw it otherwise so that we avoid all these sort of issues. I would defer if you want to go this route to you

[kb2ma]

I actually like the strict checking best to remove the ambiguity.

[jaomaloy]

Makes sense. Added below.

if not isinstance(run_detached, bool) and run_detached is not None:
            raise ValueError(f"run_detached must be True, False, or None, got {type(run_detached)}: {run_detached}")
action_api_version = "v2" if run_detached is True else self.__settings.get("device_actions_endpoint_version")

[kb2ma]

I don't see why we need to allow the value to be None. We have a default value of False. How about:

if not isinstance(run_detached, bool):
    raise ValueError(f"run_detached must be True or False, got {type(run_detached)}: {run_detached}")
action_api_version = "v2" if run_detached else self.__settings.get("device_actions_endpoint_version")

[kb2ma]

Tested HUP against live device with and without runDetached=True, and it worked as expected.

Added a commit with some simple input tests on start_os_update().

@jaomaloy if you can squash in my commit and fix the issue flagged by otaviojacoby, I'm happy to approve. Of course let me know if you have any concerns about the test I added.