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

Add owner_id to extractors #44

Merged
merged 14 commits into from
Mar 20, 2024
Merged

Add owner_id to extractors #44

merged 14 commits into from
Mar 20, 2024

Conversation

ccurme
Copy link
Collaborator

@ccurme ccurme commented Mar 19, 2024

#43

Here we just add the column.

@ccurme ccurme requested review from eyurtsev and bracesproul March 19, 2024 19:12
@@ -22,6 +22,10 @@ class CreateExtractor(BaseModel):

name: str = Field(default="", description="The name of the extractor.")

owner_id: UUID = Field(
Copy link
Collaborator

Choose a reason for hiding this comment

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

We want this information to come from the request header

@@ -188,7 +188,7 @@ async def extract_entire_document(
text_splitter = TokenTextSplitter(
chunk_size=get_chunk_size(model_name),
chunk_overlap=20,
model_name=model_name,
model_name=DEFAULT_MODEL,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unrelated bug. for now we'll use gpt 3.5 tokenizer everywhere.

@ccurme ccurme changed the title Add owner_id to extractors (wip) Add owner_id to extractors Mar 19, 2024
@ccurme ccurme changed the title (wip) Add owner_id to extractors Add owner_id to extractors Mar 19, 2024
) -> Extractor:
"""Validate the extractor id."""
extractor = (
session.query(Extractor).filter_by(uuid=extractor_id, owner_id=owner_id).first()
Copy link
Collaborator

Choose a reason for hiding this comment

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

.first() -> should usually be .scalar() or .one() if there are unique constraints

) -> List[Any]:
"""Endpoint to get all examples."""
if not validate_extractor_owner(session, extractor_id, owner_id):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This works

@eyurtsev
Copy link
Collaborator

Looks great! Merge whenever

@ccurme ccurme merged commit 474981b into main Mar 20, 2024
@ccurme ccurme deleted the cc/users branch March 20, 2024 12:14
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