Add support for Vespa Node's and indexing with additional fields #13356
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dosubot
bot
added
the
size:L
thomasht86
approved these changes
May 8, 2024
| node = metadata_dict_to_node(metadata) | ||
| text = response_fields.get("body", "") | ||
| node.set_content(text) | ||
| node = self._vespa_hit_to_node(hit) |
There was a problem hiding this comment.
This was a nice refactor! 🥇
Comment on lines
+26
to
+30
| try: | ||
| # Try getting the local instance if available | ||
| return VespaVectorStore(url="http://localhost", application_package=app_package, deployment_target="local") | ||
| except ConnectionError: | ||
| return VespaVectorStore(application_package=app_package, deployment_target="local") |
There was a problem hiding this comment.
Much better, as this allows for faster development, as you said 👍
Comment on lines
+98
to
+100
| # I am not sure what to name this, so I decided a name that highlights the difference with hybrid_template | ||
| # This needs to be renamed to something more meaningful | ||
| with_fields_template = ApplicationPackage( |
There was a problem hiding this comment.
I think this is good for now, and we can improve template naming once we get a better feel for use cases.
|
Hi great you liked it! I am overwhelmed with work on my side rn so can't say for sure but will try if I get some free time on my hands 🤟 |
|
|
||
|
|
||
| class VespaNode(TextNode): | ||
| vespa_fields: dict |
There was a problem hiding this comment.
Did we need a new object class for this? I wonder if this could just be a metadata field?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds support for indexing nodes to vespa with custom fields other than text and metadata.
Issue #13319
Motivation
A schema in Vespa has many fields that can be searched from and referenced in ranking profiles. This PR enables users to define the values for their vespa fields with a VespaNode.
Type of Change
How Has This Been Tested?
Suggested Checklist:
make format; make lintto appease the lint godsNotes
Development decisions
Future Considerations
Note to maintainers
Please don't merge this before @thomasht86 reviews and confirms the changes