Fix - disk list size

[azzamsa]

• Fix listing disk: disk object has no size attribute, but sizeMb.

Related: vmware-archive/vcd-cli#527

---

This change is 

[shashim22]

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @azzamsa)

---

pyvcloud/vcd/utils.py, line 552 at r1 (raw file):

    result['id'] = extract_id(disk.get('id'))
    result['status'] = disk.get('status')
    result['size'] = humanfriendly.format_size(int(disk.get('sizeMb')))

This change is introduced in API version 33.0. For older versions, you still need to get it from 'size' attribute. Please add a check to read correct attribute depending on API version.

Also requesting to test it manually with both API version 33.0 and 32.0.

[azzamsa]

Thanks for the input.

To get the api version, I need to do something like:

    size_key = 'size'
    if client.get_api_version() < ApiVersion.VERSION_33.value:
        size_key = 'sizeMb'

because disk object doesn't contain any information about api version. So now disk_to_dict will need two parameters client and disk.

Any cleaner solution?

[shashim22]

We can first check 'sizeMb' if it doesn't have a valid value, read 'size'. Since this is a mandatory parameter, either of the two will have a valid value. This way, we can skip checking API version.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @azzamsa)

[azzamsa]

We can first check 'sizeMb' if it doesn't have a valid value, read 'size'. Since this is a mandatory parameter, either of the two will have a valid value. This way, we can skip checking API version.

Done.

[shashim22]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @azzamsa and @shashim22)

---

pyvcloud/vcd/utils.py, line 552 at r2 (raw file):

    result['id'] = extract_id(disk.get('id'))
    result['status'] = disk.get('status')
    if 'sizeMb' in disk.keys():

I think you missed to assign 'size' to size_key before the IF block. Also, in case when 'sizeMb' is set, you will have to multiple by 1024 to get the size in bytes.

[azzamsa]

the correct formula would be:

  size_in_bytes = int(disk.get('sizeMb')) * (1000 * 1000)

Tested and compared the size using vcloud director and this lib.

[shashim22]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @azzamsa and @shashim22)

---

pyvcloud/vcd/utils.py, line 552 at r2 (raw file):

Previously, shashim22 wrote…

I think you missed to assign 'size' to size_key before the IF block. Also, in case when 'sizeMb' is set, you will have to multiple by 1024 to get the size in bytes.

Thanks for pointing it out, that was a typo. We should use 1024*1024 to be precise.
In the IF block, size_in_bytes should be converted to int:
size_in_bytes = int(disk.get('size'))

[azzamsa]

In vcloud director

using 1000 * 1000 (API result)

"size": "953 MB",
"size_bytes": "953000000",

using 1024 * 1024 (API result)

"size": "999.29 MB",
"size_bytes": "999292928",

How?

[shashim22]

https://pypi.org/project/humanfriendly/

humanfriendly.format_size(bytes, binary=True) converts the size accurately. If you don't provide binary=True, it uses 1000 in place of 1024.

Try to look at other methods in vdc.py, you can see that we have used 1024*1024 at multiple places.
vCloud director converts the same way. For example, if you give 2000 MB as disk size, it will show 1.86 GB.

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @azzamsa and @shashim22)

[shashim22]

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @azzamsa)

[shashim22]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @azzamsa)

[shashim22]

Please mark open issues as resolved so that I can merge the code.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @azzamsa)

[azzamsa]

[azzamsa]

---

pyvcloud/vcd/utils.py, line 552 at r2 (raw file):

Previously, shashim22 wrote…

Thanks for pointing it out, that was a typo. We should use 1024*1024 to be precise.
In the IF block, size_in_bytes should be converted to int:
size_in_bytes = int(disk.get('size'))

Done.

[shashim22]

Reviewable status: complete! all files reviewed, all discussions resolved