Skip to content

Conversation

raunakab
Copy link
Contributor

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.

@raunakab raunakab requested a review from Orbital-Web June 11, 2025 18:20
@raunakab raunakab requested a review from a team as a code owner June 11, 2025 18:20
Copy link

vercel bot commented Jun 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
internal-search ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 12, 2025 5:54am

@raunakab raunakab changed the title feat: Edit logic for default entity-types population Cleanup: Edit logic for default entity-types population Jun 11, 2025
@raunakab raunakab changed the title Cleanup: Edit logic for default entity-types population cleanup: Edit logic for default entity-types population Jun 11, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a 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 in backend/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

@raunakab raunakab enabled auto-merge June 11, 2025 18:21
Copy link
Contributor

@Orbital-Web Orbital-Web left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few comments

@raunakab raunakab requested a review from Orbital-Web June 12, 2025 01:00
Copy link
Contributor

@Orbital-Web Orbital-Web left a 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

@raunakab raunakab requested a review from Orbital-Web June 12, 2025 05:19
Copy link
Contributor

@Orbital-Web Orbital-Web left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good 👍

@raunakab raunakab disabled auto-merge June 12, 2025 15:46
@raunakab raunakab merged commit e689e14 into main Jun 12, 2025
10 of 11 checks passed
@raunakab raunakab deleted the feat/default-entity-types branch June 12, 2025 15:46
AnkitTukatek pushed a commit to TukaTek/onyx that referenced this pull request Sep 23, 2025
…#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
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.

2 participants