slfan1989 commented on code in PR #8641:
URL: https://github.com/apache/hadoop/pull/8641#discussion_r4091959734


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/queue-management/utils/capacityValidation.ts:
##########
@@ -15,11 +34,249 @@ import { buildPropertyKey } from '~/utils/propertyUtils';
 import { validateQueue } from '~/features/validation/service';
 import type { ValidationIssue, StagedChange, SchedulerInfo } from '~/types';
 
+const ACCESSIBLE_NODE_LABELS_PROPERTY = 'accessible-node-labels';
+const LABEL_PARTITION_ACCESS_RULE = 'label-partition-access';
+const LABEL_PARTITION_CAPACITY_REQUIRES_ACCESS_RULE =
+  'label-partition-capacity-requires-access';
+
+type AccessibleLabelAccess = 'all' | 'none' | Set<string>;
+
+export type QueuePropertyReader = Pick<SchedulerStore, 'hasQueueProperty' | 
'getQueuePropertyValue'>;
+
+export const parseAccessibleNodeLabels = (value: string): 
AccessibleLabelAccess => {
+  const trimmed = value.trim();
+  if (!trimmed) {
+    return 'none';
+  }
+  if (trimmed === '*') {
+    return 'all';
+  }
+  return new Set(trimmed.split(',').map((label) => 
label.trim()).filter(Boolean));
+};
+
+export const isLabelInAccessibleList = (
+  label: string,
+  accessibleLabels: AccessibleLabelAccess,
+): boolean => {
+  if (accessibleLabels === 'all') {
+    return true;
+  }
+  if (accessibleLabels === 'none') {
+    return false;
+  }
+  return accessibleLabels.has(label);
+};
+
+const parseQueueAccessibleNodeLabelsProperty = (
+  queuePath: string,
+  store: QueuePropertyReader,
+): AccessibleLabelAccess | null => {
+  if (!store.hasQueueProperty(queuePath, ACCESSIBLE_NODE_LABELS_PROPERTY)) {
+    return null;
+  }
+
+  return parseAccessibleNodeLabels(
+    store.getQueuePropertyValue(queuePath, 
ACCESSIBLE_NODE_LABELS_PROPERTY).value,
+  );
+};
+
+/**
+ * Returns whether a queue lists the label in its own accessible-node-labels 
property.
+ */
+export function isLabelListedInQueue(
+  queuePath: string,
+  label: string,
+  store: QueuePropertyReader,
+): boolean {
+  const accessibleLabels = parseQueueAccessibleNodeLabelsProperty(queuePath, 
store);
+  if (accessibleLabels === null) {

Review Comment:
   Could this validation account for inherited accessible labels, consistent 
with the backend's `isLabelAccessibleByQueue()` ?
   
   For example, if `root.team` explicitly grants access to `gpu` and 
`root.team.child` leaves `accessible-node-labels` unset, the child inherits 
`gpu` access. The backend accepts this configuration, but this function returns 
`false`, causing the capacity editor to reject an otherwise valid change.
   
   Please distinguish an unset property (inherit from the parent) from an 
explicitly empty property (no labeled partition access), and add a test for 
inherited access.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/queue-management/hooks/useCapacityEditor.ts:
##########
@@ -20,6 +20,7 @@
 import { SPECIAL_VALUES } from '~/types';
 import { useSchedulerStore } from '~/stores/schedulerStore';
 import type { CapacityEditorOrigin } from 
'~/stores/slices/capacityEditorSlice';
+import { a } from 'vitest/dist/chunks/suite.d.FvehnV49.js';

Review Comment:
   Please remove this unused import from Vitest's internal declaration chunk.
   
   `a` is never used and violates the configured 
`@typescript-eslint/no-unused-vars` rule. Production code should also avoid 
depending on a hashed internal path under `vitest/dist/chunks`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to