Skip to content

Commit

Permalink
update memory units (#3338)
Browse files Browse the repository at this point in the history
update tests

apply concatenation to units in workbenches table view

update parsing logic

fix TS errors

revert to text component to initial content formatting

apply Christian feedback and fix test

format

PR feedback from Christian, add unit tests

update test

return value

update getPvcTotalSize

wrap return value in  formatMemory

fix import order

fix type errors

format

update to use generic type
  • Loading branch information
jenny-s51 authored Oct 18, 2024
1 parent 448ed59 commit 3b5a56b
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe('ClusterStorage', () => {
addClusterStorageModal.findPVSizeInput().should('have.value', '19');
addClusterStorageModal.findPVSizePlusButton().click();
addClusterStorageModal.findPVSizeInput().should('have.value', '20');
addClusterStorageModal.selectPVSize('Mi');
addClusterStorageModal.selectPVSize('MiB');

//connect workbench
addClusterStorageModal
Expand Down Expand Up @@ -386,7 +386,7 @@ describe('ClusterStorage', () => {
clusterStorageRow.findKebabAction('Edit storage').click();
updateClusterStorageModal.findNameInput().should('have.value', 'Test Storage');
updateClusterStorageModal.findPVSizeInput().should('have.value', '5');
updateClusterStorageModal.shouldHavePVSizeSelectValue('Gi');
updateClusterStorageModal.shouldHavePVSizeSelectValue('GiB');
updateClusterStorageModal.findPersistentStorageWarning().should('exist');
updateClusterStorageModal.findSubmitButton().should('be.enabled');
updateClusterStorageModal.findNameInput().fill('test-updated');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ describe('Workbench page', () => {
createSpawnerPage.findPVSizeInput().should('have.value', '19');
createSpawnerPage.findPVSizePlusButton().click();
createSpawnerPage.findPVSizeInput().should('have.value', '20');
createSpawnerPage.selectPVSize('Mi');
createSpawnerPage.selectPVSize('MiB');

//add existing cluster storage
createSpawnerPage.findExistingStorageRadio().click();
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/pages/projects/components/StorageSizeBars.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const StorageSizeBar: React.FC<StorageSizeBarProps> = ({ pvc }) => {
);
}

const inUseValue = `${bytesAsRoundedGiB(inUseInBytes)}Gi`;
const inUseValue = `${bytesAsRoundedGiB(inUseInBytes)}GiB`;
const percentage = ((parseFloat(inUseValue) / parseFloat(maxValue)) * 100).toFixed(2);
const percentageLabel = error ? '' : `Storage is ${percentage}% full`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
DescriptionListTerm,
} from '@patternfly/react-core';
import { NotebookSize } from '~/types';
import { formatMemory } from '~/utilities/valueUnits';

type NotebookSizeDetailsProps = {
notebookSize: NotebookSize;
Expand All @@ -21,13 +22,13 @@ const NotebookSizeDetails: React.FC<NotebookSizeDetailsProps> = ({ notebookSize
<DescriptionListGroup>
<DescriptionListTerm>Limits</DescriptionListTerm>
<DescriptionListDescription>
{limits?.cpu ?? 'Unknown'} CPU, {limits?.memory ?? 'Unknown'} Memory
{limits?.cpu ?? 'Unknown'} CPU, {formatMemory(limits?.memory) || 'Unknown'} Memory
</DescriptionListDescription>
</DescriptionListGroup>
<DescriptionListGroup>
<DescriptionListTerm>Requests</DescriptionListTerm>
<DescriptionListDescription>
{requests?.cpu ?? 'Unknown'} CPU, {requests?.memory ?? 'Unknown'} Memory
{requests?.cpu ?? 'Unknown'} CPU, {formatMemory(requests?.memory) || 'Unknown'} Memory
</DescriptionListDescription>
</DescriptionListGroup>
</DescriptionList>
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/pages/projects/utils.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { NotebookKind, PersistentVolumeClaimKind } from '~/k8sTypes';
import { NotebookSize } from '~/types';
import { formatMemory } from '~/utilities/valueUnits';
import { NotebookState } from './notebook/types';

export const getNotebookStatusPriority = (notebookState: NotebookState): number =>
notebookState.isRunning ? 1 : notebookState.isStarting ? 2 : 3;

export const getPvcTotalSize = (pvc: PersistentVolumeClaimKind): string =>
pvc.status?.capacity?.storage || pvc.spec.resources.requests.storage;
formatMemory(pvc.status?.capacity?.storage || pvc.spec.resources.requests.storage);

export const getCustomNotebookSize = (
existingNotebook: NotebookKind | undefined,
Expand Down
25 changes: 25 additions & 0 deletions frontend/src/utilities/__tests__/valueUnits.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
isLarger,
convertToUnit,
MEMORY_UNITS_FOR_PARSING,
formatMemory,
} from '~/utilities/valueUnits';

describe('splitValueUnit', () => {
Expand Down Expand Up @@ -189,3 +190,27 @@ describe('isMemoryLimitLarger', () => {
expect(isMemoryLimitLarger(undefined, undefined)).toBe(false);
});
});

describe('formatMemory', () => {
it('should return undefined if no value is provided', () => {
expect(formatMemory(undefined)).toBeUndefined();
});

it('should return the original value if it cannot be parsed', () => {
expect(formatMemory('invalidValue')).toEqual('invalidValue');
});

it('should return the formatted value for valid inputs', () => {
expect(formatMemory('1Mi')).toEqual('1MiB');
expect(formatMemory('2Gi')).toEqual('2GiB');
expect(formatMemory('3Mi')).toEqual('3MiB');
expect(formatMemory('4Gi')).toEqual('4GiB');
expect(formatMemory('1.5Mi')).toEqual('1.5MiB');
expect(formatMemory('2.5Gi')).toEqual('2.5GiB');
});

it('should handle cases with no unit', () => {
expect(formatMemory('1')).toEqual('1');
expect(formatMemory('1.5')).toEqual('1.5');
});
});
20 changes: 18 additions & 2 deletions frontend/src/utilities/valueUnits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ export const CPU_UNITS: UnitOption[] = [
{ name: 'Milicores', unit: 'm', weight: 1 },
];
export const MEMORY_UNITS_FOR_SELECTION: UnitOption[] = [
{ name: 'Gi', unit: 'Gi', weight: 1024 },
{ name: 'Mi', unit: 'Mi', weight: 1 },
{ name: 'GiB', unit: 'Gi', weight: 1024 },
{ name: 'MiB', unit: 'Mi', weight: 1 },
];
export const MEMORY_UNITS_FOR_PARSING: UnitOption[] = [
{ name: 'EB', unit: 'E', weight: 1000 ** 6 },
Expand Down Expand Up @@ -155,3 +155,19 @@ export const isMemoryLimitLarger = (

return isLarger(limitMemory, requestMemory, MEMORY_UNITS_FOR_PARSING, isEqualOkay);
};

export const formatMemory = <T extends ValueUnitString | undefined>(
value: T,
): T | ValueUnitString => {
if (!value) {
return value;
}

const match = value.match(/^(\d*\.?\d*)(.*)$/);
if (!(match && match[1] && match[2])) {
return value;
}
return `${match[1]}${
MEMORY_UNITS_FOR_PARSING.find((o) => o.unit === match[2])?.name || match[2]
}`;
};

0 comments on commit 3b5a56b

Please sign in to comment.