update ucsfirmware.py/def firmware_add_remote#33
update ucsfirmware.py/def firmware_add_remote#33jimliming wants to merge 3 commits intoCiscoUcs:masterfrom jimliming:cleanup-firmware_add_remote
Conversation
fixed example, and removed local filepath check. This is adding a remote source, so it fails when checking os.path.exists(filepath)
|
|
||
| if not os.path.exists(file_path): | ||
| raise IOError("Image does not exist") | ||
|
|
There was a problem hiding this comment.
@jimliming thanks for the PR. May I know why check for existence of the file is not required?
There was a problem hiding this comment.
Sorry for not providing more detail.
in this function, os.path.exists doesn't provide any value. It's looking locally, when we're downloading from a remote source.
it seems we have a few options:
-
add functionality to connect to remote sources and verify the file exists before we submit the firmwareDownloader object. This seems beyond the scope of these samples.
-
submit to ucsm, and query the status of the download from ucsm.
(example of failure)
firmware_add_remote(handle, file_name="doesntexist", remote_path="/", protocol="ftp", server="ip_address", user="user", pwd="password")
file_name="doesntexist"
print (h.query_dn(dn='sys/fw-catalogue/dnld-%s' % file_name)).transfer_state
failed
(example of success)
file_name="ucs-k9-bundle-infra.2.2.5b.A.bin"
print (h.query_dn(dn='sys/fw-catalogue/dnld-%s' % file_name)).transfer_state
downloaded
- we leave this feature out of the firmware_add_remote function. it should be separate.
my PR assumes option 3. However, option 2 seems reasonable. I added the functionality.
There was a problem hiding this comment.
@jimliming looks good to me
@ragupta-git can you also review ?
|
|
||
| def return_download_state(handle, file_name): | ||
| ''' | ||
| Returns Transfer Status of FirmwareDownloader object for a single download. |
| file_name (string): firmware image name | ||
|
|
||
| Example: | ||
| return_download_status(handle, 'ucs-k9-bundle-infra.2.2.5b.A.bin') |
There was a problem hiding this comment.
return_download_status -> return_download_state
| Example: | ||
| return_download_status(handle, 'ucs-k9-bundle-infra.2.2.5b.A.bin') | ||
| ''' | ||
| download_state = (h.query_dn(dn='sys/fw-catalogue/dnld-%s' % file_name)).transfer_state |
There was a problem hiding this comment.
h -> handle.
Check and raise exception UcsOperationError if handle.query_dn returns None.
| handle.commit() | ||
|
|
||
| time.sleep(5) | ||
| if return_download_state(handle, file_name) == 'failed': |
There was a problem hiding this comment.
Better poll for TRANSFER_STATE_DOWNLOADED and raise exception if TRANSFER_STATE_FAILED.
@vvb - should we add timeout here?
| image_names = [image.image_name for image in images] | ||
| return sorted(image_names) | ||
|
|
||
| def return_download_state(handle, file_name): |
There was a problem hiding this comment.
Don't think we need this function just for querying state.
Can be a part of the firmware_add_remote function itself.
…has time to respond with a proper fail. Maybe watch until there is progress and timeout within a window?
3 participants
fixed example, and removed local filepath check. This is adding a remote source, so it always fails when checking os.path.exists(file_path).