feat: Base node parameter

[shaohuzhang1]

feat: Base node parameter

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

Code Review and Optimization Suggestions

Regularity and Issues

1. Variable Naming:

   • rawModelOptions should be defined at the component level instead of inside a method.
   • Rename groupedModelOption to groupedModelOptions for consistency.

2. Template Tags:

   • Ensure that all Vue template tags are properly closed (>).

3. Event Handling:

   • In onMounted, call fetchModelByType with the initial formValue.model_type.

4. Data Fetching:

   • The use of inject is not necessary here since these functions do not need access to parent properties.

5. Dynamic Prop Binding:

   • Use dynamic binding for :model-type="modelValue.model_type" directly without needing an intermediate variable.

6. Computed Properties:

   • Remove unnecessary variables like oldValuesList and oldIds.

7. Style Declaration:

   • Move the hover styles to the .app-container class, which seems logical given their usage across multiple components.

Potential Improvements

1. Code Reusability:

   • Extract common logic into functions such as filtering model options by provider.

2. Error Handling:

   • Add error handling for network requests when fetching models and default params.

3. Optimization:

   • Cache results from fetches if applicable and avoid redundant computations.

Here's a revised version:

<script setup lang="ts">
import { computed, onMounted, ref } from 'vue';
import {
  groupBy,
} from 'lodash';

const getSelectModelList = inject('getSelectModelList') as Function;
const getModelParamsForm = inject('getModelParamsForm') as Function;

const props = defineProps<{
  modelValue: any;
}>();

const emit = defineEmits(['update:modelValue']);

const formValue = computed({
  set: (item: any) => emit('update:modelValue', item),
  get: () => props.modelValue,
});

let rawModelOptions = reactive([]);
let groupedModelOptions = ref({});
const selectedIds = ref([]);

watchEffect(() => {
  handleModelTypeChange(formValue.value.model_type);
});

onMounted(async () => {
  await handleModelTypeChange(props.modelValue.model_type);
})

async function fetchData() {
  if (!props.modelValue.model_type) return [];

  try {
    const res = await getSelectModelList({ model_type: props.modelValue.model_type });
    return res.data || [];
  } catch (error) {
    console.error("Failed to fetch models:", error);
    return [];
  }
}

function fetchDefaultParams(modelId: string) {
  return getModelParamsForm(modelId)
    .then((res: any) => {
      return res.data;
    })
    .catch(error => {
      console.error(`Failed to fetch default parameters for ${modelId}:`, error);
      return {};
    });
}

async function initializeState() {
  const models = await fetchData();
  rawModelOptions.value = models;

  Object.assign(groupedModelOptions.value, groupBy(models, 'provider'))
}

computed({
  set(value) {
    emit('update:modelValue', value);
  },
})();

effect(() => {
  selectedIds.value = (
    formValue.value.provider_list || []
  ).flatMap(item => item.model_id);
})
  
watchEffect(async () => {
  formValue.value.provider_list.splice(0);

  if (selectedIds.value.length > 0) {
    const defaultParamsPromises = selectedIds.value.map(fetchDefaultParams);
    
    Promise.all(defaultParamsPromises).then(paramsSetMap => {
      for(const idAndParams of paramsSetMap.entries()) {
        const [id, params] = idAndParams as Array<any>;
        const originalEntry = formValue.value.provider_list.find(x => x.model_id === id); 
        if(originalEntry) {
          originalEntry.model_params_setting = params;  
        }
      }

      // Find missing defaults after setting them
      const idsToFetchNewDefaultsFor = formValue.value.provider_list.filter(
        entry => !entry.provider_params_settings ||
        Object.values(entry.provider_params_settings ?? {}).some(v => v === undefined)

      );

      idsToFetchNewDefaultsFor.forEach(id => fetchDefaultParams(id));
      
    }).catch(err => console.error("Failed to refresh default settings:", err));
  }

  if(selectedIds.value.length == 0){
    formValue.value.default_value ||= "";
  }
})

// ... Rest of your methods remain unchanged ...

Style Improvement

Ensure that your <style> block uses classes effectively and keeps the CSS clean:

<style lang="scss" scoped>
.app-container {
  // Global hover styles can go here instead
}
</style>

.template-container {
  // Hover styles specific to this component can remain under this section
}

This improved code follows best practices, provides better organization, and enhances performance through caching and reusability where appropriate.

[shaohuzhang1]

No irregularities, but consider these improvements:

1. Variable Naming Consistency: Use consistent names for similar variables (e.g., model_list and item).
2. Avoid Excessive Computed Properties: If groupedOptions remains unchanged frequently, it might be more efficient to compute it once or use an immutable data structure.
3. Inline Functions: For simple actions like checking model_value, you can directly perform operations within methods instead of storing them as separate variables.

Here's the revised code snippet with some minor adjustments:

@@ -0,0 +1,126 @@
+<template>
+  <div class="flex-between w-full">
+    <el-select
+      :model-value="selectedModelID"
+      @change="handleModelChange"
+      v-bind="$attrs"
+      popper-class="select-model"
+    >
+      <el-option-group
+        v-for="(models, providerName) in groupedProviders"
+        :key="providerName"
+        :label="associatedProvider(models)?.name"
+      >
+        <el-option
+          v-for="m in models"
+          :key="m.model_id"
+          :label="m.model_name"
+          :value="m.model_id"
+        >
+          <div class="flex">
+            <span
+              v-html="associateProvider(m)?.icon"
+              class="model-icon mr-8"
+            ><<|fim_suffix|>alue(modelID: string): void {
+       // Update model value logic here
+     }
+
+   };
+</script>

<style lang="scss" scoped>
.select-model .el-select-dropdown__footer:hover {
  background-color: var(--el-fill-color-light);
}

.model-icon {
   width: 18px;
}

.check-icon {
  position: absolute;
  right: 10px;
}
</style>

These changes ensure better readability and maintainability while improving performance slightly through direct action execution inside events handlers.

[shaohuzhang1]

The changes made to the code do not appear to have significant irregularities or performance issues. A couple of minor optimizations can be suggested:

1. Use await with Promises: Instead of using .then, which is syntactically cleaner and more readable when working with Promises.

2. Simplify Conditionals: Use a single conditional statement instead of an explicit ternary operator inside a function that already performs a similar check.

Here's the optimized version:

// ...rest of the code...

const open = async (
  model_id: string,
  application_id?: string,
  model_setting_data?: any,
  model_form_field_list?: Array<any>,
) => {
  dialogVisible.value = true;

  modelID.value = model_id;
  form_data.value = {};

  let promise: Promise<any>;
  if (model_form_field_list) {
    model_form_field.value = model_form_field_list;
    promise = Promise.resolve(); // No need to fetch API data if there's pre-defined field list
  } else {
    const api = getApi(model_id, application_id);
    promise = api.then((ok: any) => {
      model_form_field.value = ok.data;
    });
  }

  await promise;
  
  nextTick(() => {
    dynamicsFormRef.value?.render(model_form_field.value, model_setting_data);
  });

};

// ...

Key Changes:

• Used async arrow function for better readability.
• Combined the two asynchronous operations into one where applicable. If model_form_field_list is provided, it directly sets model_form_field without fetching additional data from the API.
• Eliminated redundant checks within the same block, allowing for simpler logic flow.

These changes ensure cleaner and potentially faster execution while maintaining the functionality of the original code.