feat: Knowledge base workflow supports terminating execution

[shaohuzhang1]

feat: Knowledge base workflow supports terminating execution

[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]

The code provided looks generally clean and functional. However, there are a few minor improvements and checks that can be made:

1. Button Placement: Ensure the "cancel" button is properly positioned and readable within the table column. Consider adjusting its styling or placement if necessary.

2. Event Handling:

   • The toDetails function should have proper parameter validation ensure it receives an object with expected properties.
   • Add error handling in the cancel function to manage any potential errors from the API call.

3. Documentation and Comments: Adding more documentation and comments can help clarify the purpose of each part of the codebase. This includes explaining how buttons interact with backend operations and what actions they trigger.

Here are some specific suggestions with examples:

<template>
  <!-- ... existing template content ... -->
</template>

<script>
import { ref } from 'vue';
// Import other components and utilities here

export default {
  setup() {
    // Existing variables and methods ...

    const loadSharedApi = (params: any) => {
      // Function implementation ...
    };

    const cancel = async (row: any) => {
      try {
        await loadSharedApi({ type: 'knowledge', systemType: apiType.value }).cancelWorkflowAction(
          active_knowledge_id.value,
          row.id,
          loading
        );
        // Handle successful cancellation if needed
      } catch (error) {
        console.error('Error cancelling workflow:', error);
        $message.error($t('common.errors.generic'));
      }
    };

    return {
      // ..., existing data and functions ...
      toDetails,
      cancel,
    };
  },
};
</script>

<style scoped>
/* ... existing CSS styles ... */
.flex {
  display: flex;
  justify-content: space-between;
}
</style>

By focusing on improving event handling and adding documentation, the code will be more robust and easier to maintain.

[shaohuzhang1]

The provided Python code snippet seems to be part of an API for managing knowledge workflows in a workspace. It includes two views: get and Cancel. Below is a brief analysis and some optimizations/considerations:

Analysis

1. Endpoint /api/knowledge/workflow/<workspace_id>/<knowledge_id>:

   • This endpoint uses the DetailView pattern, which typically handles GET requests to fetch a specific resource (e.g., a document).
   • The method signature indicates it takes request, workspace_id, knowledge_id, and knowledge_action_id as arguments.

2. Class Cancel:

   • Uses the APIView, allowing both POST and other HTTP methods but likely only supports POST at this level.
   • Extends Django REST Framework's @extend_schema decorator to define Swagger/OpenAPI schema information.
   • Requires appropriate permissions using can_has_permissions.
   • Cancels a knowledge workflow action with given ID.

3. Class KnowledgeWorkflowView:

   • Also extends APIView.
   • Similar functionality to the root get method but likely handles more generic knowledge workflow operations.

Optimizations/Suggestions

1. Model Method Redundancy:

   • In get view, you're creating multiple querysets (data dictionaries) before returning results. Instead, consider retrieving all necessary data in one go and then passing those into the serializer directly.

2. Method Naming Consistency:

   • In the post method of Cancel class, call .cancel() on an instance returned from .operate(). Ensure consistency here too; if possible, chain these calls together instead of storing intermediate variables.

3. Validation and Error Handling:

   • Consider adding input validation checks in get or before initiating the cancel process to ensure correct IDs are being passed.
   • Add error handling around database queries and method executions within the viewset.

4. Consistent Response Formatting:

   • Always use result.success() method consistently throughout the module to format success responses uniformly. Ensure that DefaultResultSerializer formats correctly as expected.

5. Code Readability:

   • Use clear naming conventions across classes and functions for better readability and maintainability.

Overall, while the code appears functional, improving its efficiency, clarity, and robustness through proper separation of concerns and adherence to best practices would significantly enhance its quality.

[shaohuzhang1]

I will review the code for potential issues and suggest improvements. Here's a brief summary of my findings:

1. Cache Version Import

from maxkb.const import CONFIG

Instead, it should be:
```python
from django.core.cache import cache

2. Pagination Query Set Method

• The method page does not use Django's built-in Page functionality but directly queries all items into a list before returning. This can be inefficient for large datasets.

Replace page_search() with a pagination-aware queryset.
For example:

def page(self, current_page: int, page_size: int, instance: Dict) -> QuerySet[Dict]:
    query_set = (
        QuerySet(KnowledgeAction)
        .filter(knowledge_id=self.data.get('knowledge_id'))
        .annotate(run_time_datetime=F('run_time').cast(DateTZDateTimeField()))
        .values('id', 'knowledge_id', "state", 'meta', 'details', 'run_time_datetime')
    )
    
    # Optionally add ordering based on 'run_time_datetime' or another relevant column(s)
    
    Paginator(query_set, page_size).get_page(current_page)

3. Conditional Field Lookup in Action Method

• There are multiple instances where fields like run_time, create_time, and others are retrieved conditionally (e.g., using .get()) which could cause problems if those values don't exist.

Consider handling these cases more safely, e.g., using .first() instead when only checking existence and defaulting to None/empty dict/slice.

4. Meta Validation in Action Methods

• In both action and cancel, there are direct calls to queryset.update(...). While this works, validation should ideally occur separately from execution to ensure robustness.

Move metadata updates into a separate function after ensuring the necessary permissions and conditions are met.

5. Exception Handling

• Ensure appropriate exception handling throughout the workflow methods (one, cancel, etc.). Catch exceptions properly and return meaningful error responses to avoid silent failures.

Example:

try:
    response_data = some_function()
except SomeError as e:
    raise AppApiException(str(e)) from e

General Recommendations

1. DRY Principle: Identify repeated patterns across similar methods such as querying, validating, processing, and updating records, and refactor them out into reusable functions.

   Example:

   def perform_common_operations(action_instance):
       meta_to_update = {key: value for key, value in self.validated_data.items()}
       
       knowledge_action_id = action_instance.id
       cache.set(Cache_Version.KNOWLEDGE_WORKFLOW_INTERRUPTED.get_key(action_id=knowledge_action_id), True, version=Cache_Version.KNOWLEDGE_WORKFLWO_INTERRUPTED.get_version())
       
       updated_fields = ['details', 'status']  # List of fields you want to update here
       for field in updated_fields:
           setattr(knowledge_instance, field, meta_to_update[field])
     
     # Commit changes
     knowledge_instance.save(update_fields=['details', 'status'])

2. Security Considerations: Always validate incoming data thoroughly and sanitize it appropriately to prevent injection attacks.

   Use libraries like Pydantic for schema-based validation rather than raw JSON serialisation/deserialisation.

3. Testing: Incorporate appropriate unit tests for each method that handles significant business logic steps. Test edge cases and boundary conditions thoroughly, including invalid inputs.

By addressing these concerns, your API becomes more robust, efficient, and secure.