Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions ucsmsdk_samples/firmware/ucsfirmware.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,15 +357,10 @@ def firmware_add_remote(handle, file_name, remote_path, protocol, server,
file_name="ucs-k9-bundle-c-series.2.2.5b.C.bin",
remote_path="/home/imagedir",
protocol="nfs",
server="10.65.1.2,
server="10.65.1.2",
user="user", pwd="pwd")
"""

file_path = os.path.join(remote_path, file_name)

if not os.path.exists(file_path):
raise IOError("Image does not exist")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jimliming thanks for the PR. May I know why check for existence of the file is not required?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

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

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jimliming looks good to me
@ragupta-git can you also review ?

if protocol is not FirmwareDownloaderConsts.PROTOCOL_TFTP:
if not user:
raise ValueError("Provide user")
Expand Down