This PR is to add comments to totalFileNumsInternal in pkg\ddc\alluxio\ufs_internal.go.#5882
This PR is to add comments to totalFileNumsInternal in pkg\ddc\alluxio\ufs_internal.go.#5882cah54321 wants to merge 2 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @cah54321. Thanks for your PR. I'm waiting for a fluid-cloudnative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds documentation comments to the freeStorageBytesInternal, totalStorageBytesInternal, and totalFileNumsInternal functions within the Alluxio engine. The review feedback suggests enhancing these comments by explicitly noting that freeStorageBytesInternal is currently a placeholder and refining the description of totalStorageBytesInternal to 'total size' to more accurately represent its data-counting implementation.
| return | ||
| } | ||
|
|
||
| // freeStorageBytesInternal returns the number of bytes currently available in Alluxio storage. |
There was a problem hiding this comment.
The comment for freeStorageBytesInternal is incomplete as it does not mention that the function is currently a placeholder returning (0, nil). For consistency with the neighboring usedStorageBytesInternal function, it should explicitly state its placeholder status to avoid misleading future developers.
// freeStorageBytesInternal returns the number of bytes currently available in Alluxio storage.
// It currently returns (0, nil) as a placeholder; the actual implementation should query
// the Alluxio cluster to compute the free storage capacity.| return | ||
| } | ||
|
|
||
| // totalStorageBytesInternal returns the total capacity in bytes of the Alluxio file system. |
There was a problem hiding this comment.
The term 'total capacity' in the comment is misleading. The implementation uses fileUitls.Count("/"), which returns the aggregate size of all files (data volume) in the file system, rather than the storage capacity (the upper limit of the storage medium). Describing it as the 'total size' would be more accurate.
| // totalStorageBytesInternal returns the total capacity in bytes of the Alluxio file system. | |
| // totalStorageBytesInternal returns the total size in bytes of the data in the Alluxio file system. |
|
👋 DCO check failed for this PR (action_required). Please ensure all commits include the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5882 +/- ##
==========================================
+ Coverage 61.22% 61.65% +0.42%
==========================================
Files 444 480 +36
Lines 30557 32613 +2056
==========================================
+ Hits 18710 20108 +1398
- Misses 10307 10897 +590
- Partials 1540 1608 +68 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Ⅰ. Describe what this PR does
This PR is to add comments to totalFileNumsInternal in pkg\ddc\alluxio\ufs_internal.go.
Ⅱ. Does this pull request fix one issue?
fixes #5881
Ⅲ. Special notes for reviews
This is a corrected version. In addition to commenting on the required totalFileNumsInternal function, comments have also been added for the freeStorageBytesInternal function and totalStorageBytesInternal function.