Skip to content

Cog 337 llama index support #186

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
borisarzentar merged 21 commits into from
Nov 17, 2024
Merged

Cog 337 llama index support #186

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
ec5fd80
feat: Add support for LlamaIndex Document type
dexters1 Nov 6, 2024
b05a07f
docs: Add Jupyer Notebook for cognee with llama index document type
dexters1 Nov 6, 2024
93445bf
feat: Add metadata migration from LlamaIndex document type
dexters1 Nov 7, 2024
804ca26
refactor: Change llama index migration function name
dexters1 Nov 7, 2024
c151d7b
chore: Add llama index core dependency
dexters1 Nov 7, 2024
fb84aa0
Merge branch 'COG-337-llama-index-support' of github.com:topoteretes/…
dexters1 Nov 7, 2024
7d222ee
Feature: Add ingest_data_with_metadata task
dexters1 Nov 7, 2024
094e45d
docs: Add description on why specific type checking is done
dexters1 Nov 7, 2024
d5aa316
fix: Add missing parameter to function call
dexters1 Nov 7, 2024
98dc18c
refactor: Move storing of data from async to sync function
dexters1 Nov 7, 2024
2615b35
refactor: Pretend ingest_data was changes instead of having two tasks
dexters1 Nov 7, 2024
e4c39e8
refactor: Use old name for data ingestion with metadata
dexters1 Nov 7, 2024
733caa7
refactor: Return ingest_data and save_data_to_storage Tasks
dexters1 Nov 8, 2024
8c7fdc4
refactor: Return previous ingestion Tasks to add function
dexters1 Nov 8, 2024
f13146d
Merge branch 'main' of github.com:topoteretes/cognee into COG-337-lla…
dexters1 Nov 14, 2024
cc53693
fix: Remove dict and use string for search query
dexters1 Nov 14, 2024
af99e4f
refactor: Add changes request in pull request
dexters1 Nov 15, 2024
417b119
Merge branch 'main' of github.com:topoteretes/cognee into COG-337-lla…
dexters1 Nov 15, 2024
4c7a4e2
fix: Resolve issue with llama-index being mandatory
dexters1 Nov 15, 2024
c4700b1
fix: Add install of llama-index to notebook
dexters1 Nov 15, 2024
f6e8294
Merge branch 'main' into COG-337-llama-index-support
borisarzentar Nov 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
Merge branch 'main' of github.com:topoteretes/cognee into COG-337-lla… 
…ma-index-support
  • Loading branch information
@dexters1
dexters1 committed Nov 14, 2024
commit f13146d62347a0b1525f05af0f27c089bb2e36d2
Binary file removed .DS_Store
View file
Open in desktop
Binary file not shown.
63 changes: 63 additions & 0 deletions .github/workflows/test_cognee_llama_index_notebook.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
name: test | llama index notebook

on:
workflow_dispatch:
pull_request:
branches:
- main
types: [labeled]

@0xideas 0xideas Nov 14, 2024

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.

currently, we apply [labeled, synchronize] in the other GitHub actions



concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true

env:
RUNTIME__LOG_LEVEL: ERROR

jobs:
get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml

run_notebook_test:
name: test
needs: get_docs_changes
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true' && ${{ github.event.label.name == 'run-checks' }}
runs-on: ubuntu-latest
Comment on lines +18 to +27

@coderabbitai coderabbitai Bot Nov 14, 2024 •
edited
Loading

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.

⚠️ Potential issue

Fix the condition syntax in the if statement.

The current condition mixes template syntax unnecessarily, which could cause evaluation issues.

-    if: needs.get_docs_changes.outputs.changes_outside_docs == 'true' && ${{ github.event.label.name == 'run-checks' }}
+    if: needs.get_docs_changes.outputs.changes_outside_docs == 'true' && github.event.label.name == 'run-checks'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
jobs:
get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml
run_notebook_test:
name: test
needs: get_docs_changes
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true' && ${{ github.event.label.name == 'run-checks' }}
runs-on: ubuntu-latest
jobs:
get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml
run_notebook_test:
name: test
needs: get_docs_changes
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true' && github.event.label.name == 'run-checks'
runs-on: ubuntu-latest
🧰 Tools
🪛 actionlint

26-26: if: condition "needs.get_docs_changes.outputs.changes_outside_docs == 'true' && ${{ github.event.label.name == 'run-checks' }}" is always evaluated to true because extra characters are around ${{ }}

(if-cond)

@borisarzentar borisarzentar Nov 14, 2024

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.

@dexters1 This

@coderabbitai coderabbitai Bot Nov 14, 2024

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.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

@dexters1 dexters1 Nov 15, 2024

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@borisarzentar Added change, maybe we should change this for the rest of the actions as well?

defaults:
run:
shell: bash
steps:
- name: Check out
uses: actions/checkout@master

- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: '3.11.x'

- name: Install Poetry
uses: snok/install-poetry@v1.3.2
with:
virtualenvs-create: true
virtualenvs-in-project: true
installer-parallel: true

- name: Install dependencies
run: |
poetry install --no-interaction
poetry add jupyter --no-interaction

Comment on lines +35 to +51

@coderabbitai coderabbitai Bot Nov 15, 2024

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.

🛠️ Refactor suggestion

Add Poetry caching for faster workflow execution.

Consider these improvements for the Python/Poetry setup:

  1. Add Poetry cache to speed up dependency installation
  2. Pin Poetry version more specifically for better reproducibility
 - name: Setup Python
   uses: actions/setup-python@v5
   with:
     python-version: '3.11.x'

+- name: Cache Poetry
+  uses: actions/cache@v3
+  with:
+    path: ~/.cache/pypoetry
+    key: ${{ runner.os }}-poetry-${{ hashFiles('**/poetry.lock') }}
+    restore-keys: |
+      ${{ runner.os }}-poetry-

 - name: Install Poetry
-  uses: snok/install-poetry@v1.3.2
+  uses: snok/install-poetry@v1.3.3
   with:
     virtualenvs-create: true
     virtualenvs-in-project: true
     installer-parallel: true
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: '3.11.x'
- name: Install Poetry
uses: snok/install-poetry@v1.3.2
with:
virtualenvs-create: true
virtualenvs-in-project: true
installer-parallel: true
- name: Install dependencies
run: |
poetry install --no-interaction --all-extras --no-root
poetry add jupyter --no-interaction
- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: '3.11.x'
- name: Cache Poetry
uses: actions/cache@v3
with:
path: ~/.cache/pypoetry
key: ${{ runner.os }}-poetry-${{ hashFiles('**/poetry.lock') }}
restore-keys: |
${{ runner.os }}-poetry-
- name: Install Poetry
uses: snok/install-poetry@v1.3.3
with:
virtualenvs-create: true
virtualenvs-in-project: true
installer-parallel: true
- name: Install dependencies
run: |
poetry install --no-interaction --all-extras --no-root
poetry add jupyter --no-interaction

- name: Execute Jupyter Notebook
env:
ENV: 'dev'
LLM_API_KEY: ${{ secrets.OPENAI_API_KEY }}
GRAPHISTRY_USERNAME: ${{ secrets.GRAPHISTRY_USERNAME }}
GRAPHISTRY_PASSWORD: ${{ secrets.GRAPHISTRY_PASSWORD }}
run: |
poetry run jupyter nbconvert \
--to notebook \
--execute notebooks/cognee_llama_index.ipynb \
--output executed_notebook.ipynb \
--ExecutePreprocessor.timeout=1200
Comment on lines +52 to +63

@coderabbitai coderabbitai Bot Nov 15, 2024

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.

🛠️ Refactor suggestion

Enhance notebook execution error handling and fix EOF.

  1. The 20-minute timeout suggests potential performance concerns. Consider adding error handling and output capture.
  2. Add a newline at the end of file.
       run: |
         poetry run jupyter nbconvert \
         --to notebook \
         --execute notebooks/cognee_llama_index.ipynb \
         --output executed_notebook.ipynb \
-        --ExecutePreprocessor.timeout=1200
+        --ExecutePreprocessor.timeout=1200 \
+        || {
+          echo "::error::Notebook execution failed"
+          cat executed_notebook.ipynb
+          exit 1
+        }
+
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Execute Jupyter Notebook
env:
ENV: 'dev'
LLM_API_KEY: ${{ secrets.OPENAI_API_KEY }}
GRAPHISTRY_USERNAME: ${{ secrets.GRAPHISTRY_USERNAME }}
GRAPHISTRY_PASSWORD: ${{ secrets.GRAPHISTRY_PASSWORD }}
run: |
poetry run jupyter nbconvert \
--to notebook \
--execute notebooks/cognee_llama_index.ipynb \
--output executed_notebook.ipynb \
--ExecutePreprocessor.timeout=1200
- name: Execute Jupyter Notebook
env:
ENV: 'dev'
LLM_API_KEY: ${{ secrets.OPENAI_API_KEY }}
GRAPHISTRY_USERNAME: ${{ secrets.GRAPHISTRY_USERNAME }}
GRAPHISTRY_PASSWORD: ${{ secrets.GRAPHISTRY_PASSWORD }}
run: |
poetry run jupyter nbconvert \
--to notebook \
--execute notebooks/cognee_llama_index.ipynb \
--output executed_notebook.ipynb \
--ExecutePreprocessor.timeout=1200 \
|| {
echo "::error::Notebook execution failed"
cat executed_notebook.ipynb
exit 1
}
🧰 Tools
🪛 yamllint

[error] 63-63: no new line character at the end of file

(new-line-at-end-of-file)

Binary file removed cognee/.DS_Store
View file
Open in desktop
Binary file not shown.
9 changes: 7 additions & 2 deletions poetry.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

You are viewing a condensed version of this merge commit. You can view the full changes here.