Skip to content

Conversation

@ssh-esh
Copy link
Collaborator

@ssh-esh ssh-esh commented Oct 16, 2025

Issues
Closes #79
Closes #81

Description
Note: Apologies to the reviewers, in hindsight this should have been probably two separate PRs. If it becomes difficult to review then I will split it and close this PR. I will try to split the concerns by commits for easy review

The lock file changes seem quite extensive but I think it is due to poetry being upgraded from 1.84 -> 2.1.4. The old lock file is incompatible with the new client it seems so i had to do a poetry lock command to generate this lock file 🤔

@ssh-esh ssh-esh changed the title Update client version and tests Update client version, update tests for new retriever format and update auth flow Oct 17, 2025
@ssh-esh ssh-esh marked this pull request as ready for review October 17, 2025 00:42
@ssh-esh
Copy link
Collaborator Author

ssh-esh commented Oct 17, 2025

We also need to figure out what type of release to do for this. The suggestion from Enrico was to do a 0.4.0 release although this would be a major version change we are still in 0.X

@ssh-esh
Copy link
Collaborator Author

ssh-esh commented Oct 17, 2025

@miguelgrinberg a point that came out of talking with @ezimuel about this PR was if we really need a commit the poetry lock file to this repo since this is more of a library than a deployable application. I think the repo was using client version 8.15.1 even though we defined it as ^8.15.1 because the lock file used an old version of poetry that is incompatible with newer versions of the client. What do you think?

@miguelgrinberg
Copy link
Collaborator

miguelgrinberg commented Oct 20, 2025

@ssh-esh The lockfile does not need to be in the repo, I agree. But removing the lockfile does not address the problem. Correct me if I'm wrong, but if you say ^8.15.1 in pyproject.toml, that means that you are asking for the latest 8.15.x release. You can change this to ^8.15 and then you would be asking for the latest 8.x.x release, which is better, but still leaves the 9.x.x versions out. My suggestion to use >= and < for bounds checking seems to match our needs better.

@ssh-esh
Copy link
Collaborator Author

ssh-esh commented Oct 20, 2025

monas

@ssh-esh The lockfile does not need to be in the repo, I agree. But removing the lockfile does not address the problem. Correct me if I'm wrong, but if you say ^8.15.1 in pyproject.toml, that means that you are asking for the latest 8.15.x release. You can change this to ^8.15 and then you would be asking for the latest 8.x.x release, which is better, but still leaves the 9.x.x versions out. My suggestion to use >= and < for bounds checking seems to match our needs better.

Yes absolutely the lock file was a separate problem and will not address the main issue here

Based on yours and @ezimuel comments here is my understanding of next steps:

  • I make a separate PR from this and we release langchain-elastic 0.3.3 with elasticsearch-py <8.16.0 (or just pin it maybe to 8.15.1?) so that anyone who wants to use the old flow can have the client that send the requests without retriever
  • We follow up with this PR and release langchain-elastic 0.4.0 with elasticsearch-py ">=8.16 <9" for users to use the new retriever format and remove the dead RRF code
  • We remove the poetry lock file in this PR

Please let me know if I have missed anything and my understanding is clear 👍

@miguelgrinberg
Copy link
Collaborator

miguelgrinberg commented Oct 20, 2025

@ssh-esh I apologise if you mentioned this before and I forgot, but what is the versioning problem you are trying to address? As I mentioned before, the logic in the Python client is able to identify when the old or new RRF style has to be used on the fly. We've looked at this long ago. The only real problem I see is that we are not allowing people to use the v9 client, and that has no easy solution except waiting until everyone migrates to the v9 server and we can change the dependency to >=9 <10.

Update: after reading the poetry docs on caret version dependencies, the ^8.15.1 that we are using is the same as >=8.15.1 <9, so it is effectively the same I'm proposing. So I really do not understand what is the problem with the current system, except that caret dependencies are a poetry-specific thing that Python itself does not support, so it is a bad idea to use them.

@ssh-esh ssh-esh force-pushed the update-client-version-and-tests branch from 6348aac to 37d8b73 Compare October 21, 2025 16:54
@ssh-esh
Copy link
Collaborator Author

ssh-esh commented Oct 21, 2025

Summary of changes since the last comment made by @miguelgrinberg :

  • remove of the lock file
  • add lock file to git ignore
  • update elasticsearch client version range
  • consolidate dependancies from lock file into the the pyproject.toml. We are pinning versions instead of downloading latest .git as this would require us to update python dependancies and other dependancies. v1 langchain was released a few days ago
    • langchain version pin to "^0.3.10"
    • langchain-community pin to "^0.3.10"
    • langchain-text-splitters pin to "^0.3.0"
    • Remove langchain-core from sections like [tool.poetry.group.test.dependencies] as this is already defined in the main [tool.poetry.dependencies] so it will be installed already
  • Remove a now redundant check for the lock file in the workflow inside file _lint.yml

@ssh-esh
Copy link
Collaborator Author

ssh-esh commented Oct 22, 2025

Summary of changes since the last comment made by @miguelgrinberg :

  • remove of the lock file

  • add lock file to git ignore

  • update elasticsearch client version range

  • consolidate dependancies from lock file into the the pyproject.toml. We are pinning versions instead of downloading latest .git as this would require us to update python dependancies and other dependancies. v1 langchain was released a few days ago

    • langchain version pin to "^0.3.10"
    • langchain-community pin to "^0.3.10"
    • langchain-text-splitters pin to "^0.3.0"
    • Remove langchain-core from sections like [tool.poetry.group.test.dependencies] as this is already defined in the main [tool.poetry.dependencies] so it will be installed already
  • Remove a now redundant check for the lock file in the workflow inside file _lint.yml

Adding more flexibility for the versions here instead of carats, i am using >= on the same version numbers and allow poetry to resolve the highest possible versions. It will not install v1 langchain as that requires a minimum python version of 3.10,right now we support 3.9. It should be enough for a 0.4.0 release and then we will support v1 langchain when we release v1 langchain-elastic 👍

@ssh-esh
Copy link
Collaborator Author

ssh-esh commented Oct 23, 2025

After some feedback the last few commits will

  • update langchain version range to langchain = ">=0.3.10,<1.0.0"
  • remove langchain_community and langchain_text_splitters from the test dependancies. community can be substituted in our usage by langchain_core and text splitters is automatically installed with 'langchain'
  • Use langchain_core for FakeMEssagesListChatModel in tests
  • add DistanceStrategy import to init
  • update spynhix docs in ElasticSearchStore to not use langchain_community

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix auth issues for integeration tests Use retriever instead of rank parameter for hybrid search in ES 9

3 participants