Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ public class ServiceOfferingJoinDaoImpl extends GenericDaoBase<ServiceOfferingJo

private SearchBuilder<ServiceOfferingJoinVO> sofIdSearch;

/**
* Constant used to convert GB into Bytes (or the other way around).
* GB * MB * KB = Bytes //
* 1024 * 1024 * 1024 = 1073741824
*/
private static final long GB_TO_BYTES = 1073741824;
Copy link
Contributor

Choose a reason for hiding this comment

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

multiple constants are defined, in various classes, for the value "1024 * 1024 * 1024", and it is hardcoded in some cases. may be (later), a constant can be defined in common package( eg. cloud utils), and use it across. (Such common util functions has to be documented, so that the developers can use these)

Also, some get disk size calls return size in Bytes, and others return in GB. I think, it is better to explicitly mention (in variables / methods) it indicating Bytes or GB for non-default unit (i.e. If Bytes is the default unit, then methods which return in 'GB' should have it mentioned). If there is no such default unit, then variables / methods should indicate the size units explicitly. This can be a huge refactoring work (so can be skipped for now) and at least, have to make sure new code indicates proper size unit.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sureshanaparti I totally agree with you. We had many issues in the past and it still haunts us. If we simply added a proper name and documentation it would help a lot many bugs that we face and also make new features and extensions easier on that matter.

Regarding the constant. You have a good point; we could create constants/enums for such conversions as many times we store in Bytes but make it available to users in GB or MB.


protected ServiceOfferingJoinDaoImpl() {

sofIdSearch = createSearchBuilder();
Expand Down Expand Up @@ -123,7 +130,8 @@ public ServiceOfferingResponse newServiceOfferingResponse(ServiceOfferingJoinVO
}
}

offeringResponse.setRootDiskSize(offering.getRootDiskSize());
long rootDiskSizeInGb = (long) offering.getRootDiskSize() / GB_TO_BYTES;
offeringResponse.setRootDiskSize(rootDiskSizeInGb);

return offeringResponse;
}
Expand Down
2 changes: 1 addition & 1 deletion 2 ui/src/components/view/DetailsTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
</div>
<div v-else-if="$route.meta.name === 'computeoffering' && item === 'rootdisksize'">
<div>
{{ parseFloat( resource.rootdisksize / (1024.0 * 1024.0 * 1024.0)).toFixed(1) }} GB
{{ resource.rootdisksize }} GB
Copy link
Member Author

Choose a reason for hiding this comment

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

@shwstppr thanks for pointing out the change necessary to keep UI aligned with API.
I believe this will fix it, please let me know if I am missing anything.

</div>
</div>
<div v-else-if="['name', 'type'].includes(item)">
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.