Skip to content

Update get_cognify_router.py with graph endpoint #372

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.

Sign up for GitHub

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

Merged
dexters1 merged 5 commits into from
Dec 17, 2024
Merged

Update get_cognify_router.py with graph endpoint #372

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dc37646
Update get_cognify_router.py
Vasilije1990 Dec 14, 2024
cfad366
update
Vasilije1990 Dec 16, 2024
3366bb3
Fixes
Vasilije1990 Dec 16, 2024
98cc84a
Fixes to our demo
Vasilije1990 Dec 16, 2024
3f72652
Merge branch 'dev' into add_model_to_endpoint
dexters1 Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
update
  • Loading branch information
@Vasilije1990
Vasilije1990 committed Dec 16, 2024
commit cfad366c5ca9c1961a2b162a0bdec3a3680ef09c
3 changes: 2 additions & 1 deletion cognee/api/v1/cognify/routers/get_cognify_router.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@

class CognifyPayloadDTO(BaseModel):
datasets: List[str]
Comment on lines 10 to 11
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 14, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing graph_model field to CognifyPayloadDTO

The DTO is missing the graph_model field which is being accessed in the implementation. This will cause runtime errors.

Apply this diff to add the required field:

 class CognifyPayloadDTO(BaseModel):
     datasets: List[str]
+    graph_model: BaseModel

Committable suggestion skipped: line range outside the PR's diff.

graph_model: BaseModel
Copy link
Collaborator

@dexters1 dexters1 Dec 16, 2024

Choose a reason for hiding this comment

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

If we want to have graph_model be optional we should add the type in an Optional wrapper

Copy link
Contributor Author

@Vasilije1990 Vasilije1990 Dec 16, 2024

Choose a reason for hiding this comment

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

fixed


def get_cognify_router() -> APIRouter:
router = APIRouter()

@router.post("/", response_model=None)
async def cognify(payload: CognifyPayloadDTO, user: User = Depends(get_authenticated_user), graph_model:BaseModel):
async def cognify(payload: CognifyPayloadDTO, user: User = Depends(get_authenticated_user), graph_model:BaseModel=None):
Copy link
Collaborator

@dexters1 dexters1 Dec 16, 2024

Choose a reason for hiding this comment

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

If graph_model is part of the CognifyPayloadDTO I don't think we need it as an input parameter in the endpoint.

Copy link
Contributor Author

@Vasilije1990 Vasilije1990 Dec 16, 2024

Choose a reason for hiding this comment

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

fixed

""" This endpoint is responsible for the cognitive processing of the content."""
from cognee.api.v1.cognify.cognify_v2 import cognify as cognee_cognify
try:
Expand Down
Loading