NSG improvements #638
NSG improvements #638
Conversation
| return HostPluginProtocol._is_default_channel | ||
|
|
||
| @staticmethod | ||
| def set_default_channel(is_default): |
There was a problem hiding this comment.
If we want to set this to True all the time(other than UT cases), do we need the argument?
There was a problem hiding this comment.
Not at the moment, but we do need it for UTs, and we may want to switch this flag off in the future.
| body = remove_bom(response.read()) | ||
| if body is None: | ||
| return '' |
There was a problem hiding this comment.
This change loses the status / reason when the body is absent. Do we want that? It seems they may be useful for other errors.
| @@ -649,6 +657,9 @@ def fetch_manifest(self, version_uris): | |||
| else: | |||
| host.manifest_uri = version.uri | |||
| logger.verbose("Manifest downloaded successfully from host plugin") | |||
| if not HostPluginProtocol.is_default_channel(): | |||
| logger.info("Setting host plugin as default channel") | |||
| HostPluginProtocol.set_default_channel(True) | |||
There was a problem hiding this comment.
You may want to push this into the HostPluginProtocol: It should become the default channel any time it is used and succeeds. Once default, it stays default, even if subsequent errors occur.
There was a problem hiding this comment.
This is ok for get_versions and put_vm_log (which we are not using yet) but the complication is that we use a common _fetch or _download outside of host plugin and simply update the url with the get_artifact_request call. At the moment that is the primary scenario too, so we cannot reliably detect host plugin usage from within HostPluginProtocol.
/cc @brendandixon @jinhyunr