#1596 Fix Dependency convergence issue + enable Maven Enforcer #37
Conversation
Dohbedoh
changed the title
langchain4j
left a comment
langchain4j
left a comment
There was a problem hiding this comment.
@Dohbedoh thank you!
| <exclusions> | ||
| <exclusion> | ||
| <groupId>ai.djl</groupId> | ||
| <artifactId>api</artifactId> |
There was a problem hiding this comment.
Shouldn't we exclude slf4j-api on ai.djl:api instead?
<dependency>
<groupId>ai.djl</groupId>
<artifactId>api</artifactId>
<version>0.28.0</version>
<exclusions>
<!-- due to CVE-2024-26308 and CVE-2024-25710-->
<exclusion>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
</exclusion>
<exclusion>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</exclusion>
</exclusions>
</dependency>There was a problem hiding this comment.
I am not following. I am not touching the exclusions for ai.djl:api https://github.com/langchain4j/langchain4j-embeddings/blob/0.33.0/langchain4j-embeddings/pom.xml#L37-L41.
The problem is that embedding has 2 dependencies on ai.djl:api, one direct dependency and a transitive dependency through ai.djl.huggingface:tokenizers. To avoid future problems, we should exclude ai.djl:api from ai.djl.huggingface:tokenizers.
There was a problem hiding this comment.
AFAIU, tokenizers always uses the same version of ai.djl:api, so there is no need in exclusions, we just need to make sure we use the same version of api and tokenizers.
But if we remove exclusion of api, there is a problem with slf4j-api...
There was a problem hiding this comment.
we just need to make sure we use the same version of api and tokenizers.
That's why I propose the exclusion here. ai.djl:api. The same thing we did for common-compress in 6c358b8.
There was a problem hiding this comment.
Excluding ai.djl:api is not necessary, it works fine without it. And maven-enforcer-plugin will always make sure there is no conflict.
langchain4j
left a comment
langchain4j
left a comment
There was a problem hiding this comment.
@Dohbedoh thank you!
| <exclusions> | ||
| <exclusion> | ||
| <groupId>ai.djl</groupId> | ||
| <artifactId>api</artifactId> |
There was a problem hiding this comment.
Excluding ai.djl:api is not necessary, it works fine without it. And maven-enforcer-plugin will always make sure there is no conflict.
Process in the same thread when embedding single segment, otherwise use executor and cache threads for 1 second (#34) ## Issue This PR partially fixes langchain4j/langchain4j#1454
2 participants
Fixes langchain4j/langchain4j#1596
This fixes the dependency convergence issues introduced in https://github.com/langchain4j/langchain4j-embeddings/pull/29/files. Since embedding manages a direct dependency on
ai.djl:apito excludecommons-compress, I propose to add an exclusion ofai.djl:apifromai.djl.huggingfaceto avoid transitive dependencies issues.Also added the enforcer to avoid this in the future.