bitflicker64 commented on code in PR #834: URL: https://github.com/apache/hugegraph-toolchain/pull/834#discussion_r4349471532
########## hugegraph-hubble/hubble-fe/src/pages/GraphSpace/storageUsage.js: ########## @@ -0,0 +1,58 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +const LEGACY_UNLIMITED = 2147483647; + +export const formatStorageUsed = item => { Review Comment: Minor: `formatStorageUsed` mostly repeats the existing `byteConvert` helper in `src/utils/format.js`, which also takes KB and steps through KB, MB, GB and TB (`Graph/Card.js` uses it). The strings differ in detail: `byteConvert` keeps trailing zeros (`1.10 MB`), shows `0 KB` for 0 and switches to TB at 1 TB, so `storageUsage.test.js` would change with it. It also drops the i18n unit that the base used for the used amount, so zh-CN now shows `512 MB / 10240G`. Suggest `byteConvert(kb)` when `storage_used_kb` is a nonnegative number, else `byteConvert((item.storage_used ?? 0) * 1024 * 1024)`, and render the limit in the same unit style. ########## hugegraph-hubble/hubble-fe/src/i18n/resources/en-US/modules/pages.json: ########## @@ -152,17 +153,19 @@ "name_help": "An optional display alias or note. It is not used for URL or authorization matching; when empty, the GraphSpace name is shown", "auth_help": "When enabled, access within this GraphSpace is checked by HugeGraph authorization. It cannot be changed after creation", "max_graph_help": "The maximum number of graphs allowed in this GraphSpace. Defaults to 100", - "graph_cpu_help": "CPU limit for graph read and write services. Defaults to 64 vCPU", + "graph_cpu_help": "CPU limit for graph read and write services. Defaults to 128 vCPU", "graph_memory_help": "Memory limit for graph read and write services. Defaults to 128 GB", - "compute_cpu_help": "CPU limit for asynchronous compute tasks. Defaults to 64 vCPU", - "compute_memory_help": "Memory limit for asynchronous compute tasks. Defaults to 128 GB", + "compute_cpu_help": "CPU limit for asynchronous compute tasks. The form defaults to 128 vCPU; 0 continuously inherits the graph-service CPU limit", + "compute_memory_help": "Memory limit for asynchronous compute tasks. The form defaults to 128 GB; 0 continuously inherits the graph-service memory limit", "oltp_namespace_help": "Kubernetes namespace for graph read and write services. Leave blank outside Kubernetes", "olap_namespace_help": "Kubernetes namespace for asynchronous compute tasks. Leave blank when compute tasks are not used", - "storage_limit_help": "Storage capacity limit for this GraphSpace. Defaults to 1000000 GB", + "storage_limit_help": "Storage capacity limit for this GraphSpace. Defaults to 10240 GB", "image_format_error": "Please enter a valid image address (e.g. example.com/org_1/xx_img:1.0.0)", "id_rule": "Start with a letter, lowercase letters/digits/underscore, max 48 chars", "name_rule": "Start with a letter or Chinese character; common symbols are allowed; max 48 characters", - "k8s_rule": "Start with a letter, lowercase letters/digits/hyphen, max 48 chars" + "k8s_rule": "Start with a letter, lowercase letters/digits/hyphen, max 48 chars", + "compute_inheritance_help": "Set 0 to keep inheriting the corresponding graph-service limit as it changes. Positive values stay independent. Saving preserves inheritance", Review Comment: Minor: this holds only with a Server that has apache/hugegraph#3391. Before that change, the Server update path reads `getOrDefault("compute_cpu_limit", 0)` and skips the field when it is 0. With the new `min={0}`, editing a space from 64 to 0 on an older Server reports success and keeps 64. The PR body says older Servers are supported, but only for usage. One sentence in this help text and the README saying that 0 inheritance on update needs #3391 is enough. ########## hugegraph-hubble/hubble-fe/src/pages/GraphSpace/EditLayer.js: ########## @@ -318,19 +323,25 @@ const EditLayer = ({visible, detail, onCancel, refresh}) => { </Form.Item> <Typography.Title level={5}>{t('graphspace.form.compute_task')}</Typography.Title> + <Typography.Paragraph type='secondary'> + {t('graphspace.form.compute_inheritance_help')} + </Typography.Paragraph> <MyFormItem label={t('graphspace.form.cpu')} help={t('graphspace.form.compute_cpu_help')} unit={t('graphspace.unit.cpu')} + extra={computeCpu === 0 Review Comment: Minor: the hint only appears for `=== 0`, but clearing the field also saves inheritance. InputNumber emits `null`, the form sends `"compute_cpu_limit": null`, Jackson maps it to 0 through the `GraphSpaceEntity` setter, and with #3391 an update with 0 means inherit. So a user who clears the field saves inheritance with no hint (the base saved Hubble's default 64 for the same input). Either show the hint for `computeCpu == null || computeCpu === 0` (same for memory at line 353), or make the field required. ########## hugegraph-dist/release-docs/licenses/fe-licenses/generate.py: ########## @@ -49,8 +49,9 @@ def sha256(data): def installed_packages(): expected_manager = json.loads((FRONTEND / 'package.json').read_text())['packageManager'] + # Corepack selects the pinned manager before pnpm processes its arguments. actual_manager = subprocess.check_output([ - 'pnpm', '--dir', str(FRONTEND), '--version']).decode().strip() + 'pnpm', '--version'], cwd=FRONTEND).decode().strip() Review Comment: Minor: this `cwd` fix, plus `test_generate.py` and the new CI step, is not part of the graph-space change. The same fix is also in #832, #833 and #835. Those three match each other, but this copy differs (extra comment, different wrapping, plus the test), so whichever lands first will make the others conflict on these lines. Suggest landing the fix once, in one PR or its own, and dropping it from the rest. The existing `generate.py` step already fails on the pinned-manager check if it regresses. ########## hugegraph-client/src/main/java/org/apache/hugegraph/structure/space/GraphSpace.java: ########## @@ -333,7 +344,7 @@ public Object convertReq() { return new GraphSpaceReq(this); } - @JsonIgnoreProperties({"cpu_used", "memory_used", "storage_used", + @JsonIgnoreProperties({"cpu_used", "memory_used", "storage_used", "storage_used_kb", "storage_percent", "graph_number_used", "role_number_used", "create_time", "update_time"}) public static class GraphSpaceReq extends GraphSpace { Review Comment: Minor: `GraphSpaceReq` still copies `computeCpuLimit` and `computeMemoryLimit` as `int`, so a hugegraph-client update built without them sends 0. With apache/hugegraph#3391, 0 on update means inherit, so such a partial update resets a stored 16 to inherit. Only Hubble avoids this, through the `GraphSpaceEntity` overrides. The README sentence "omitted compute fields stay omitted through the Java client" holds for Hubble, not for direct client users. Suggest moving the two provided flags into `GraphSpace` (set them in the two setters), dropping unset fields in `GraphSpace.convertReq()`, and deleting the Hubble overrides. Read-modify-write callers are unaffected, because GET returns the stored values. The same point is raised on #3391. -- 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]
