HPCC-35694 Add support to WsDFU.DFUQuery to sort by physical "File Size"#21119
HPCC-35694 Add support to WsDFU.DFUQuery to sort by physical "File Size"#21119asselitx wants to merge 1 commit intohpcc-systems:candidate-10.2.xfrom
Conversation
|
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-35694 Jirabot Action Result: |
a963176 to
4814af3
Compare
There was a problem hiding this comment.
Pull request overview
Adds backend and UI support for sorting DFUQuery results by compressed size / on-disk size semantics, and updates UI to display “File Size” and “Compression” instead of separate size columns.
Changes:
- Map DFUQuery sort field to
@compressedSizeand adjust size field population for compressed files. - Extend
DFULogicalFileto include aFileSizefield (v1.68+) and populate it in helpers. - Update React/ECLWatch UI formatting and tests to reflect “File Size” / “Compression” display.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| testing/unittests/dalitests.cpp | Adds coverage asserting DFUQuery returned size uses compressed size when appropriate. |
| esp/src/tests/v9-files.spec.ts | Updates UI test expectations for renamed columns (“File Size”, “Compression”). |
| esp/src/src/Utility.ts | Extends formatDecimal to support prefix/postfix formatting. |
| esp/src/src-react/hooks/file.ts | Adds shared compression formatting logic for UI. |
| esp/src/src-react/components/MetricsPropertiesTables.tsx | Uses new formatDecimal prefix/postfix for stddev display. |
| esp/src/src-react/components/LogicalFileSummary.tsx | Replaces PercentCompressed with computed Compression display. |
| esp/src/src-react/components/IndexFileSummary.tsx | Updates percent formatting to use new formatDecimal signature. |
| esp/src/src-react/components/Files.tsx | Updates columns to “File Size” and “Compression” and adjusts size/compression formatting. |
| esp/src/eclwatch/DFUQueryWidget.js | Updates percent formatting to use new formatDecimal signature. |
| esp/services/ws_dfu/ws_dfuService.cpp | Adds legacy sort mapping for compressed size attribute. |
| esp/services/ws_dfu/ws_dfuHelpers.cpp | Populates new FileSize (v1.68+) based on available fields. |
| esp/scm/ws_dfu_common.ecm | Adds FileSize to DFULogicalFile (min_ver 1.68). |
| dali/base/dadfs.cpp | Sets DFUQ “size” field to compressed size for compressed files and ensures origsize exists. |
| export function formatDecimal(num: number, prefix: string = "", postfix: string = ""): string { | ||
| if (!num) return ""; | ||
| if (isNaN(num)) return num.toString(); | ||
| return d3FormatDecimal(num); | ||
| return prefix + d3FormatDecimal(num) + postfix; |
| if (row.FileSize) { | ||
| return Utility.convertedSize(row.FileSize); | ||
| } | ||
| return Utility.convertedSize(row.IsCompressed ? row.CompressedFileSize : row.IntSize); | ||
| }, | ||
| csvFormatter: (value, row) => { | ||
| if (row.FileSize) { |
There was a problem hiding this comment.
Ok adding suggestion to batch.
| function formatRatio(isCompressed: boolean, compressedSize: number, totalSize: number): string { | ||
| if (!isCompressed || totalSize <= 0) return ""; | ||
| const ratio = compressedSize / totalSize; | ||
| if (!isFinite(ratio)) return ""; | ||
| return Utility.formatDecimal(100 - ratio * 100, "", "%"); | ||
| } |
🔄 Upmerge Test ResultsStatus: ✅ All branches merged successfully ✅ Successful Branches (1)
|
| [min_ver("1.63"), nil_remove] int64 MaxSkew; | ||
| [min_ver("1.63"), nil_remove] int64 MinSkewPart; | ||
| [min_ver("1.63"), nil_remove] int64 MaxSkewPart; | ||
| [min_ver("1.68")] int64 FileSize; |
There was a problem hiding this comment.
Does this need a version bump somewhere else (for the entire service)?
| label: nlsHPCC.FileSize, | ||
| formatter: (value, row) => { | ||
| return Utility.convertedSize(row.IntSize); | ||
| if (row.FileSize) { |
There was a problem hiding this comment.
row.FileSize !== undefined would be preferable.
| return Utility.convertedSize(row.IsCompressed ? row.CompressedFileSize : row.IntSize); | ||
| }, | ||
| csvFormatter: (value, row) => { | ||
| if (row.FileSize) { |
There was a problem hiding this comment.
row.FileSize !== undefined is preferable
🔄 Upmerge Test ResultsStatus: ✅ All branches merged successfully ✅ Successful Branches (1)
|
|
@GordonSmith fixed requested changes and confirmed unittests pass, but didn't touch your code that only copilot commented on. If you do agree with copilot I can make those adjustments also. |
|
@asselitx please squash |
|
Also the copilot comment about formatting 0 looks relevant. |
|
It looks like the formatDecimal (and others) intentionally return empty string for zero input to avoid cluttering the UI with |
- Expose single field 'FileSize' for physical File Size. Regardless of compression status this field has the actual physical size on disk. - Add FileSize field to DFUQuery response to hold DFUSFSize. - Update ECL Watch to use FileSize when present. - Update DFULogicalFile structure and all services using it to version 1.69. - Update unittests to return all DFU fields needed to pass new tests. Signed-off-by: Gordon Smith<GordonJSmith@gmail.com> Signed-off-by: Terrence Asselin <terrence.asselin+copilot@lexisnexisrisk.com>
ad10c43 to
9ba041a
Compare
🔄 Upmerge Test ResultsStatus: ✅ All branches merged successfully ✅ Successful Branches (1)
|
|
@ghalliday and @GordonSmith squashed and approved |
| lFile->setMinSkewPart(file.getPropInt64(getDFUQResultFieldName(DFUQResultField::minSkewPart))); | ||
| } | ||
|
|
||
| if (version >= 1.68) |
There was a problem hiding this comment.
I think this should be compare >= 1.69
Type of change:
Checklist:
Smoketest:
Testing: