← 返回首页
examples: Make RAG examples a bit more generic and demoable by antoniivanov · Pull Request #3085 · vmware/versatile-data-kit · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted
Cancel Submit feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel Create saved search
Appearance settings
Resetting focus

examples: Make RAG examples a bit more generic and demoable#3085

Merged
antoniivanov merged 4 commits into
mainfrom
person/aivanov/rag
Feb 8, 2024
Merged

examples: Make RAG examples a bit more generic and demoable#3085
antoniivanov merged 4 commits into
mainfrom
person/aivanov/rag

Conversation

antoniivanov commented Feb 6, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

For confluence-reader:

For embed-ingest-job-example:

  • parameterize the table names used in the job
  • add clean-up deleted rows (though afterwards I realised it's redundant for now as
    we need to drop the table first as the postgres ingestion does not
    support upserts (updates))
  • as the embedding job is written in so generic way. Actually, there's no
    need to tie it to confluence at all. It would work for any dataset.
  • Added multiple TODOs for missing features. The job could be even
    further generalized if our ingestion framework improves
  • renamed embed-ingest-job-exmaple ot pgvector-embedder to better show its responsibilities

duyguHsnHsn approved these changes Feb 6, 2024
View reviewed changes

duyguHsnHsn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Looks good to me! Should we create ticket for some of the todos?

yonitoo approved these changes Feb 6, 2024
View reviewed changes

yonitoo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Awesome generalization. LGTM.
A bit of a side note but I recently read this SentenceTransformer issue suggesting that the stop words removal and lemmatization (part of our text cleaning) are not needed for transformer models - passing the original text is suggested. Maybe we can remove the whole cleaning part and directly embed (could be done here or as part of another story).

antoniivanov commented Feb 6, 2024

Copy link
Copy Markdown
Contributor Author

Looks good to me! Should we create ticket for some of the todos?

Not yet. As we build a backlog for the next milestones, then we will create the needed tickets.

antoniivanov commented Feb 6, 2024

Copy link
Copy Markdown
Contributor Author

Awesome generalization. LGTM. A bit of a side note but I recently read this SentenceTransformer issue suggesting that the stop words removal and lemmatization (part of our text cleaning) are not needed for transformer models - passing the original text is suggested. Maybe we can remove the whole cleaning part and directly embed (could be done here or as part of another story).

Ok. Maybe separately. But I also want to have some cleaning logic, because it's something you would expect ot have in a pipeline and we need to figure out how to handle it properly.

examples: confluence-reader: fixes to be more demoable …
d7436bd
- the recursive method for find pages was crashing so repalced with more CQL - added passing parent id so we can take only few pages for demo purpsoes - Noted bugs and issues in the code and added todos
embed-ingest-job-example: make more demoable …
6fc7cd8
- parameterize the table names used in the job - add clean up deleted rows (though I realised it's redundant for now as we need to drop the table first as the postgres ingestion does not support upserts (updates)) - as the embedding job is written in so generic way. Actually there's no need to tie it to confluence at all. It would work for any dataset. - Added multiple TODOs for missing features. The job could be even further generalied if our ingestion frameowrk improves
renamed embed-ingest-job-example to pgvector-embedder
182771c
a few more fixes
cceb5da
yonitoo mentioned this pull request Feb 7, 2024
Closed
antoniivanov changed the title Make RAG examples a bit more generic and demoable examples: Make RAG examples a bit more generic and demoable Feb 8, 2024
antoniivanov merged commit 62961c7 into main Feb 8, 2024
antoniivanov deleted the person/aivanov/rag branch February 8, 2024 13:22
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
yonitoo yonitoo approved these changes
duyguHsnHsn duyguHsnHsn approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla-not-required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Footer

© 2026 GitHub, Inc.