Skip to content

Conversation

@wolfdancer
Copy link
Owner

No description provided.

@wolfdancer wolfdancer requested a review from Copilot April 6, 2025 05:03
@wolfdancer wolfdancer self-assigned this Apr 6, 2025
@wolfdancer wolfdancer linked an issue Apr 6, 2025 that may be closed by this pull request
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment on lines 15 to 17
def __init__(self, docs_dir, embedding_config={}):
self.docs_dir = docs_dir
self.embedding_type = embedding_type
self.embedding_config = embedding_config or {}
self.embedding_config = embedding_config
Copy link

Copilot AI Apr 6, 2025

Choose a reason for hiding this comment

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

Using a mutable default argument for embedding_config can lead to unexpected behavior. Consider using None as the default value and initializing with {} inside the method.

Copilot uses AI. Check for mistakes.
@wolfdancer wolfdancer requested a review from Copilot April 6, 2025 05:06
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

# Only import if needed
from langchain_openai import OpenAIEmbeddings
api_key = self.embedding_config.get("api_key", os.getenv("OPENAI_API_KEY"))
api_key = self.embedding_config["api_key"]
Copy link

Copilot AI Apr 6, 2025

Choose a reason for hiding this comment

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

Removing the fallback using os.getenv may lead to a KeyError when the API key is not provided. Consider restoring fallback behavior by using self.embedding_config.get('api_key', os.getenv('OPENAI_API_KEY')).

Suggested change
api_key = self.embedding_config["api_key"]
api_key = self.embedding_config.get('api_key', os.getenv('OPENAI_API_KEY'))

Copilot uses AI. Check for mistakes.
@wolfdancer wolfdancer merged commit 4887740 into main Apr 6, 2025
1 check passed
@wolfdancer wolfdancer deleted the 35-document_processor-does-not-look-to-be-using-openai branch April 6, 2025 05:18
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.

document_processor does not look to be using OpenAI?

2 participants