Skip to content

Conversation

@jaalah-akamai
Copy link
Contributor

Description 📝

We need to revert a previous PR that changed the OBJ conversions from base2 to base10. Object Storage size conversions need to use base2 calculations even though we display base10 units (MB/GB/TB).

Changes 🔄

  • Reverts back to base2 for readableBytes usage
  • Add comments for clarity
  • Adds clearer tests to hopefully avoid confusion in the future

Target release date 🗓️

5/6 or earlier if out-of-cycle release

How to test 🧪

Verification steps

Object Size:

  • Navigate to Object Storage > Create a bucket
  • Upload files with specific sizes to test conversions:
    • Upload a file that's exactly 1024 bytes
    • Should display as "1 KB" (not "1.02 KB")
    • Upload a file that's exactly 1048576 bytes (1024 * 1024)
    • Should display as "1 MB" (not "1.05 MB")

Bucket Size:

  • Create multiple buckets with various file sizes
  • Check the bucket table and confirm:
    • Sizes are displayed with proper base2 calculations
    • Total usage calculations use base2 math

Edge Case Testing:

  • Upload a file that's 1000 bytes
  • Should display as "1000 bytes" (not "1 KB")
  • Upload a file that's 1,000,000 bytes
  • Should display as "976.56 KB" or "0.95 MB" (not "1 MB")
Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support


  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@jaalah-akamai jaalah-akamai self-assigned this Apr 21, 2025
@jaalah-akamai jaalah-akamai added Bug Fixes for regressions or bugs Object Storage deals with Object Storage labels Apr 21, 2025
@jaalah-akamai jaalah-akamai marked this pull request as ready for review April 21, 2025 20:18
@jaalah-akamai jaalah-akamai requested a review from a team as a code owner April 21, 2025 20:18
@jaalah-akamai jaalah-akamai requested review from abailly-akamai, mjac0bs and pmakode-akamai and removed request for a team April 21, 2025 20:18
}

export type StorageSymbol = 'GB' | 'KB' | 'MB' | 'TB' | 'byte' | 'bytes';
export type StorageSymbol = 'byte' | 'bytes' | 'GB' | 'KB' | 'MB' | 'TB';
Copy link
Contributor

Choose a reason for hiding this comment

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

Couple questions here:

  1. are we not receiving/displaying actual base2 metric names (GiB, KiB, etc). I believe this was an important consideration
  2. could those types be moved to APIv4 package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. That is correct - That's an intentional decision from Product which appears to be in alignment with other industry practices
  2. Potentially! Let me look

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure it belongs in APIv4 - open to be convinced
On a side note, I think I'm going to follow-up on this PR and try to combine some redundancy from the Cloud Pulse utils packages/manager/src/features/CloudPulse/Utils/unitConversion.ts

Copy link
Contributor

Choose a reason for hiding this comment

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

fair enough, i suppose API usually sends everything in bytes so yeah, maybe not. It feels more right to import this type from there (which I need to do) but not a big deal.

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Looked right based on my understanding of this. Verified...

Object size:

  • Displays as 1KB for 1024 byte files
  • Displays as 1MB for 1,048,576 byte files
  • Displays as 1000 bytes for 1000 byte files
  • Displays as 977KB (rounded) for 1,000,000 byte files
Prod This Branch
Screenshot 2025-04-22 at 8 47 02 AM Screenshot 2025-04-22 at 8 45 20 AM

Files of various sizes created with truncate command:
Screenshot 2025-04-22 at 8 46 00 AM

Bucket size:

  • Sums the sizes of objects in the buckets correctly
    • e.g. bucket with two 1000 byte objects displays total size in 1.95KB

@jaalah-akamai jaalah-akamai added the Add'tl Approval Needed Waiting on another approval! label Apr 22, 2025
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 565 passing tests on test run #4 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing565 Passing5 Skipped106m 22s

Copy link
Contributor

@pmakode-akamai pmakode-akamai left a comment

Choose a reason for hiding this comment

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

Looks good ✅

  • Object Size:
    Screenshot 2025-04-23 at 2 20 26 PM

  • Bucket Size:

    • bucket-test-1 (files of 1024 bytes + 1048576 bytes + 1000 bytes + 1,000,000 bytes = displays 1.96MB)
    • bucket-test-2 (files of 1024 bytes + 1000 bytes = displays 1.98 KB)
      Screenshot 2025-04-23 at 2 30 29 PM

@github-project-automation github-project-automation bot moved this from Review to Approved in Cloud Manager Apr 23, 2025
@pmakode-akamai pmakode-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Apr 23, 2025
Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

Confirmed conversion works as expected and and confirmed the GB VS GiB (base2/base10) conversion is accurate and the nomenclature is expected (although I still find it weird we're not displaying GiB, TiB etc

@jaalah-akamai jaalah-akamai merged commit 0a323a6 into linode:develop Apr 23, 2025
35 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Merged in Cloud Manager Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! Bug Fixes for regressions or bugs Object Storage deals with Object Storage

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants