Skip to content
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

Updating Enrichment status #1544

Merged
merged 16 commits into from
Oct 31, 2024
Merged

Updating Enrichment status #1544

merged 16 commits into from
Oct 31, 2024

Conversation

shreyaspimpalgaonkar
Copy link
Contributor

@shreyaspimpalgaonkar shreyaspimpalgaonkar commented Oct 31, 2024

Important

Update enrichment and extraction status handling across workflows, services, and database interactions, including new status types and endpoint in management_router.py.

  • Behavior:
    • Update handling of kg_enrichment_status and kg_extraction_status in kg_workflow.py, ingestion_workflow.py, and simple/kg_workflow.py.
    • Modify on_failure methods to update status to FAILED for both enrichment and extraction workflows.
    • Add new endpoint /r2r_project_name in management_router.py to return project name from environment variable.
  • Database:
    • Add kg_enrichment_status to get_collections_overview() in collection.py.
    • Update set_workflow_status() and get_workflow_status() in document.py to handle new status types.
  • Models:
    • Add kg_enrichment_status to CollectionOverviewResponse in responses.py.
    • Add methods table_name() and id_column() to IngestionStatus, KGExtractionStatus, and KGEnrichmentStatus in document.py.
  • Misc:
    • Minor formatting and import changes in ingestion.py and database.py.

This description was created by Ellipsis for 24abc6d. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to ce73f2e in 1 minute and 55 seconds

More details
  • Looked at 623 lines of code in 11 files
  • Skipped 1 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. py/core/main/orchestration/hatchet/kg_workflow.py:295
  • Draft comment:
    Add a check to ensure input_data['kg_enrichment_settings'] is not None before accessing its attributes to avoid potential AttributeError.
  • Reason this comment was not posted:
    Marked as duplicate.
2. py/core/main/services/ingestion_service.py:541
  • Draft comment:
    Add a check to ensure chunk_enrichment_settings is not None before accessing its attributes to avoid potential AttributeError.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_dCuSIltlcbcl6xag


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

or {},
)

if chunk_enrichment_settings.enable_chunk_enrichment:
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a check to ensure chunk_enrichment_settings is not None before accessing chunk_enrichment_settings.enable_chunk_enrichment to avoid potential AttributeError.

Suggested change
if chunk_enrichment_settings.enable_chunk_enrichment:
if chunk_enrichment_settings and chunk_enrichment_settings.enable_chunk_enrichment:

@@ -61,32 +68,50 @@ async def enrich_graph(input_data):

Copy link
Contributor

Choose a reason for hiding this comment

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

Add a check to ensure input_data['kg_enrichment_settings'] is not None before accessing its attributes to avoid potential AttributeError.

return self.value

@classmethod
def table_name(cls) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate methods table_name and id_column found in the same file. Consider reusing the existing methods at lines 149-155.

@@ -887,3 +887,10 @@ async def delete_conversation(
) -> WrappedDeleteResponse:
await self.service.delete_conversation(conversation_id)
return None # type: ignore

@self.router.get("/r2r_project_name")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add this to the server settings?

Copy link

vercel bot commented Oct 31, 2024

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

Name Status Preview Comments Updated (UTC)
yc_demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 31, 2024 8:54pm
yc-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 31, 2024 8:54pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
recommendation_platform ⬜️ Ignored (Inspect) Oct 31, 2024 8:54pm

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot 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 to me! Incremental review on ef87b47 in 19 seconds

More details
  • Looked at 47 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. py/core/main/api/management_router.py:887
  • Draft comment:
    The PR description mentions adding a new endpoint /r2r_project_name, but this code shows it was removed. Please update the PR description to reflect the actual changes.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_HNBDHVF8EsKc6wKY


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot 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 to me! Incremental review on 24abc6d in 19 seconds

More details
  • Looked at 29 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. py/shared/api/models/management/responses.py:59
  • Draft comment:
    Remove the commented-out r2r_version line if it's not needed, or uncomment it if it is needed.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The commented-out line for r2r_version in AppSettingsResponse should be removed if it's not needed, or uncommented if it is needed. Leaving commented-out code can lead to confusion.
2. py/core/main/services/management_service.py:204
  • Draft comment:
    Remove the commented-out r2r_version line if it's not needed, or uncomment it if it is needed.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The commented-out line for r2r_version in app_settings method should be removed if it's not needed, or uncommented if it is needed. Leaving commented-out code can lead to confusion.

Workflow ID: wflow_fBAuJj5LdYg0gITx


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@NolanTrem NolanTrem merged commit 6fe18bd into main Oct 31, 2024
14 of 15 checks passed
@NolanTrem NolanTrem deleted the enrichment_status branch October 31, 2024 21:03
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.

3 participants