Skip to content

Camel-Milvus: create rag helpers to be used as beans#21994

Open
smongiar wants to merge 1 commit intoapache:mainfrom
smongiar:add-milvus-rag-features
Open

Camel-Milvus: create rag helpers to be used as beans#21994
smongiar wants to merge 1 commit intoapache:mainfrom
smongiar:add-milvus-rag-features

Conversation

@smongiar
Copy link
Contributor

#Description

Need to collect processors and beans for more comfortable usage in route DSL definitions. Some changes involve JBang as well to make it work with camel CLI

#Target

[ X] I checked that the commit is targeting the correct branch (Camel 4 uses the main branch)

[ X] I have run mvn clean install -DskipTests locally from root folder and I have committed all auto-generated changes.

@github-actions
Copy link
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using build-all, build-dependents, skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

private String collectionName = "rag_collection";
private String collectionDescription = "RAG collection";
private String idFieldName = "id";
private String dimension = "768";
Copy link
Contributor

Choose a reason for hiding this comment

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

why are int/long types Strings?

import io.milvus.response.QueryResultsWrapper;
import org.apache.camel.Exchange;

public class RAGResultExtractor {
Copy link
Contributor

Choose a reason for hiding this comment

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

why isn't it a processor?


FieldType vectorField = FieldType.newBuilder()
.withName(vectorFieldName)
.withDataType(DataType.FloatVector)
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be configurable?

Copy link
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

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

A few observations on this PR:

  1. Test changes modify what's being tested: The existing tests (MilvusCreateCollectionTest, MilvusComponentIT) previously tested the Milvus component directly using the Milvus Java SDK APIs. The refactored tests now route through the new RAG helpers, which means they're no longer testing the raw component behavior — they're testing the helpers + component together. This could mask issues in either layer. Consider keeping the original tests as-is and adding new tests for the RAG helpers separately.

  2. No input validation in processors: The RAG processors like RAGInsert and RAGUpsert split textFieldMappings on = and access pair[1] without bounds checking. A malformed mapping like "content" (missing =value) would throw an ArrayIndexOutOfBoundsException. Similar issue with Integer.parseInt(dimension) — invalid input gives an unhelpful error.

  3. dimension and limit as String fields: In RAGCreateCollection, dimension is stored as String but always parsed to int. Same for limit in RAGSearch and textFieldMaxLength. Using int/long fields directly would be more natural and catch invalid values at configuration time rather than at runtime.

  4. Unchecked cast warning: exchange.getIn().getBody(List.class) in RAGInsert/RAGUpsert/RAGSearch — the @SuppressWarnings("unchecked") suppresses the warning but getBody(List.class) could return null if the body isn't convertible. No null check before using the result.

These are minor issues. The overall approach of providing RAG helper beans is sound.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants