Add Spring Boot auto-configuration for Neo4j vector store #191
Conversation
There was a problem hiding this comment.
Thanks @King-Chau
It looks great! I have few minor recommendations about the code style and one important concern about exposing the org.neo4j.* in the spring-ai API.
Btw, your PR made me review the other vector stores and I've tried to clean the similar issues there.
| /** | ||
| * @author Jingzhou Ou | ||
| */ | ||
| @ConfigurationProperties(CONFIG_PREFIX) |
There was a problem hiding this comment.
Please change to @ConfigurationProperties(Neo4jVectorStoreProperties.CONFIG_PREFIX) to ged rid of the static import.
There was a problem hiding this comment.
good point, will change
| * @author Jingzhou Ou | ||
| */ | ||
| @ConfigurationProperties(CONFIG_PREFIX) | ||
| public class Neo4jDriverProperties { |
There was a problem hiding this comment.
Use Neo4jDriverProperties.CONFIG_PREFIX inside the annotation and remove the static import.
There was a problem hiding this comment.
good point, will change
|
|
||
| package org.springframework.ai.autoconfigure.neo4j; | ||
|
|
||
| import org.neo4j.driver.AuthToken; |
There was a problem hiding this comment.
It is a bad practice to expose 3rd party API (e.g. org.neo4j) directly in your own API.
Can we hide those behind spring-ai abstractions?
Also I wonder how different is this from the existing neo4j spring boot autoconfig?
There was a problem hiding this comment.
Good point. I already removed the org.neo4j.* APIs in Neo4jDriverProperties.
The properties are the same as the existing neo4j spring boot autoconfig.
|
LGTM, thank you @King-Chau |
|
Rebased, squashed and merged at: b625ab1 |
Add Spring Boot auto-configuration for Neo4j vector store