Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Fix Missing CPU and Memory Limit Warnings When Editing Installe… #7313

Merged
merged 1 commit into from
Dec 10, 2024
Merged

feat: Fix Missing CPU and Memory Limit Warnings When Editing Installe… #7313

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 45 additions & 7 deletions frontend/src/views/app-store/installed/detail/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@
<el-checkbox v-model="paramModel.allowPort" :label="$t('app.allowPort')" size="large" />
<span class="input-help">{{ $t('app.allowPortHelper') }}</span>
</el-form-item>
<el-form-item :label="$t('container.cpuQuota')" prop="cpuQuota">
<el-form-item
:label="$t('container.cpuQuota')"
prop="cpuQuota"
:rules="checkNumberRange(0, limits.cpu)"
>
<el-input
type="number"
style="width: 40%"
Expand All @@ -76,19 +80,32 @@
>
<template #append>{{ $t('app.cpuCore') }}</template>
</el-input>
<span class="input-help">{{ $t('container.limitHelper') }}</span>
<span class="input-help">
{{ $t('container.limitHelper', [limits.cpu]) }} {{ $t('commons.units.core') }}
</span>
</el-form-item>
<el-form-item :label="$t('container.memoryLimit')" prop="memoryLimit">
<el-form-item
:label="$t('container.memoryLimit')"
prop="memoryLimit"
:rules="checkNumberRange(0, limits.memory)"
>
<el-input style="width: 40%" v-model.number="paramModel.memoryLimit" maxlength="10">
<template #append>
<el-select v-model="paramModel.memoryUnit" placeholder="Select" style="width: 85px">
<el-select
v-model="paramModel.memoryUnit"
placeholder="Select"
style="width: 85px"
@change="changeUnit"
>
<el-option label="KB" value="K" />
<el-option label="MB" value="M" />
<el-option label="GB" value="G" />
</el-select>
</template>
</el-input>
<span class="input-help">{{ $t('container.limitHelper') }}</span>
<span class="input-help">
{{ $t('container.limitHelper', [limits.memory]) }} {{ paramModel.memoryUnit }}B
</span>
</el-form-item>

<el-form-item prop="editCompose">
Expand Down Expand Up @@ -137,6 +154,8 @@ import { Codemirror } from 'vue-codemirror';
import { javascript } from '@codemirror/lang-javascript';
import { oneDark } from '@codemirror/theme-one-dark';
import { getLanguage } from '@/utils/util';
import { Container } from '@/api/interface/container';
import { loadResourceLimit } from '@/api/modules/container';

const extensions = [javascript(), oneDark];

Expand All @@ -163,13 +182,32 @@ const paramModel = ref<any>({
});
const rules = reactive({
params: {},
cpuQuota: [Rules.requiredInput, checkNumberRange(0, 999)],
memoryLimit: [Rules.requiredInput, checkNumberRange(0, 9999999999)],
containerName: [Rules.containerName],
});
const submitModel = ref<any>({});
const oldMemory = ref<number>(0);
const limits = ref<Container.ResourceLimit>({
cpu: null as number,
memory: null as number,
});

const changeUnit = () => {
if (paramModel.value.memoryUnit == 'M') {
limits.value.memory = oldMemory.value;
} else {
limits.value.memory = Number((oldMemory.value / 1024).toFixed(2));
}
};

const loadLimit = async () => {
const res = await loadResourceLimit();
limits.value = res.data;
limits.value.memory = Number((limits.value.memory / 1024 / 1024).toFixed(2));
oldMemory.value = limits.value.memory;
};

const acceptParams = async (props: ParamProps) => {
loadLimit();
submitModel.value.installId = props.id;
params.value = [];
paramData.value.id = props.id;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the given snippet of code, there seem to be a few areas that could be improved. Some differences and improvements suggested:

  1. In submitModel, instead of defining both CPU and MEMORY_LIMIT with same validations and rules under a single key (params), consider using different keys as you may have specific constraints on CPU Quota vs. Memory Limit.
// ... other definitions...
const submitModel = ref<{[key: string]: any}>({cpuQuota: {}, memoryLimit: {...}}); // use proper object keys for distinct validation & rule sets
  1. The extensions array is imported twice (once before extending codemirror and again after). You can import them only once at the beginning so we don't accidentally re-import them unnecessarily later.
  2. In JavaScript language definition, it would help if these were properly formatted and structured like they should be per best practices.
  3. The documentation isn’t very clear about why codeMirrorInstance was mentioned; it seems out-of-line from rest of the code section. Consider placing it at its respective place closer to where it will be used within the component(s).
  4. There's some duplication across multiple places which might confuse others reading through this codebase. Ensure each instance has appropriate imports/uses.
  5. Lastly, make note of missing <script lang...> tags when importing external libraries/components. This prevents errors related to incorrect file inclusion in TypeScript files.

For optimization suggestion:
Use the correct syntax when importing React components from another folder into typescript to prevent error messages during compiles. For example:

import { EditorState } from "@vue/reactivity";

Replace "EditorState" in the above line with your actual component name.

This helps keeping your .tsx code more tidy without worrying about unnecessary complexity introduced due to typing mismatching between ts and js codes leading to unexpected runtime errors.

Expand Down
Loading