-
Notifications
You must be signed in to change notification settings - Fork 2k
cleanup: Edit logic for default entity-types population #4876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Refactored the Knowledge Graph entity types population to improve architectural design and return values. The changes focus on better database session management and explicit data returns.
- Function
populate_default_entity_types
inbackend/onyx/kg/setup/kg_default_entity_definitions.py
now accepts db_session as parameter instead of creating internally - Changed return type from None to list of all entity types for better data visibility
- Added explicit template constant and improved vendor name validation
- Improved separation of concerns by breaking down into smaller functions
- Enforced proper db session handling in
backend/onyx/chat/process_message.py
by explicitly passing session parameter
2 files reviewed, no comments
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
few comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few comments, otherwise lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good 👍
…#4876) * Edit logic for default entity-types population * Remove templatization in favour of function * Address comments on PR * Pull call out into its own binding * Address comments on PR
Description
This PR updates how the
populate_default_entity_types
function works internally, and what it returns. With this update, it returns all the entity-types (including the fresh, templatized defaults) which have been inserted into the DB.Addresses: https://linear.app/danswer/issue/DAN-2080/edit-how-populate-default-entity-types-works-and-what-it-returns.
How Has This Been Tested?
Not tested.