Skip to content

chore: rename files to use underscore instead of dashes #1389

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

Jump to bottom
Merged
merged 1 commit into from
Aug 13, 2025
Merged

chore: rename files to use underscore instead of dashes #1389

merged 1 commit into from
Aug 13, 2025

Conversation

Jont828
Copy link
Contributor

@Jont828 Jont828 commented Aug 13, 2025

Reason for Change:

Missed a few files in #1361

Requirements

  • added unit tests and e2e tests (if applicable).

Issue Fixed:

Notes for Reviewers:

@kaito-pr-agent Kaito PR Agent
Copy link

Title

Add Preset RAG Controller and Utilities

Description

  • Added new controller for preset RAG deployment

  • Included unit tests for preset RAG functionality

  • Introduced utility functions for common preset configurations

Changes walkthrough 📝

Relevant files
Enhancement
preset_rag.go
New preset RAG controller implementation                                 

pkg/ragengine/controllers/preset_rag.go

  • Added new file with preset RAG controller logic
  • Implemented CreatePresetRAG function to handle deployment creation
  • Defined constants and variables for deployment configuration
    		• [link]   
    common_preset.go
    Common preset utility functions                                                   

    pkg/utils/common_preset.go

  • Added new file with utility functions for common preset configurations
  • Implemented functions for configuring volumes, image pull secrets, and
    more
    		• [link]   
    Tests
    preset_rag_test.go
    Unit tests for preset RAG controller                                         

    pkg/ragengine/controllers/preset_rag_test.go

  • Added new file with unit tests for preset RAG controller
  • Implemented test cases for deployment creation and GPU configuration
    logic
    		• [link]   
    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
    		•

    @kaito-pr-agent Kaito PR Agent
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 5 🔵🔵🔵🔵🔵
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The CreatePresetRAG function ignores errors returned by resources.CreateResource. It would be better to log the error or handle it appropriately.

     
    err := resources.CreateResource(ctx, depObj, kubeClient)
if client.IgnoreAlreadyExists(err) != nil {
	return nil, err
    Mocking

    The callMocks function in the test cases uses mock.Anything for the third parameter in c.On("Create", ...). This can lead to false positives if the third parameter is important for the test. Consider specifying the exact parameters or using mock.MatchedBy for more precise matching.

     
    c.On("Create", mock.IsType(context.TODO()), mock.IsType(&appsv1.Deployment{}), mock.Anything).Return(nil)
    Environment Variable Usage

    The GetPresetImageName function relies on the PRESET_REGISTRY_NAME environment variable without providing a default value. This could lead to runtime errors if the environment variable is not set. Consider providing a default value or handling the case where the environment variable is missing.

     
    return fmt.Sprintf("%s/kaito-%s:%s",
	os.Getenv("PRESET_REGISTRY_NAME"),

    @kaito-pr-agent Kaito PR Agent
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Use named constant for medium

    Consider using a named constant for the Medium field to improve readability and
    maintainability.

    pkg/utils/common_preset.go [144-147]

     volumeSource = corev1.VolumeSource{
-  EmptyDir: &corev1.EmptyDirVolumeSource{},
+  EmptyDir: &corev1.EmptyDirVolumeSource{
+    Medium: corev1.StorageMediumMemory,
+  },
 }
    Suggestion importance[1-10]: 5

    __

    Why: Using a named constant improves code readability and maintainability, which is beneficial for future changes.

    		Low
    Explicitly set medium

    Ensure that the Medium field is explicitly set to avoid potential issues with
    default behavior changes.

    pkg/utils/common_preset.go [144-147]

     volumeSource = corev1.VolumeSource{
-  EmptyDir: &corev1.EmptyDirVolumeSource{},
+  EmptyDir: &corev1.EmptyDirVolumeSource{
+    Medium: corev1.StorageMediumMemory,
+  },
 }
    Suggestion importance[1-10]: 5

    __

    Why: Explicitly setting the Medium field helps avoid potential issues with default behavior changes, enhancing code reliability.

    		Low
    Fix trailing comma

    Remove the trailing comma to maintain Go syntax consistency.

    pkg/ragengine/controllers/preset_rag.go [40-43]

     containerPorts = []corev1.ContainerPort{{
   ContainerPort: int32(PortInferenceServer),
-},
-}
+}}
    Suggestion importance[1-10]: 3

    __

    Why: Removing the trailing comma improves Go syntax consistency, but it is a minor stylistic change with low impact.

    		Low
    Remove outdated TODO

    Remove the TODO comment once the image is updated to the MCR image.

    pkg/ragengine/controllers/preset_rag_test.go [51]

    -expectedImage: "aimodelsregistrytest.azurecr.io/kaito-rag-service:0.3.2", //TODO: Change to the mcr image when release
+expectedImage: "aimodelsregistrytest.azurecr.io/kaito-rag-service:0.3.2",
    Suggestion importance[1-10]: 3

    __

    Why: Removing the TODO comment is a minor cleanup task that does not significantly affect functionality.

    		Low

    @chewong chewong merged commit 026be28 into kaito-project:main Aug 13, 2025
    13 of 15 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @chewong chewong chewong approved these changes

    @Fei-Guo Fei-Guo Awaiting requested review from Fei-Guo Fei-Guo is a code owner

    @zhuangqh zhuangqh Awaiting requested review from zhuangqh zhuangqh is a code owner

    Assignees
    No one assigned
    Labels
    Review effort 5/5
    Projects
    KAITO Roadmap
    Status: Done
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    2 participants
    @Jont828 @chewong