Skip to content
This repository was archived by the owner on Jun 7, 2025. It is now read-only.

Conversation

@Whyle
Copy link
Contributor

@Whyle Whyle commented May 15, 2025

Dare possibilita anche di eseguire con i env opzionali

@Whyle Whyle requested review from Copilot and lucaribon May 15, 2025 23:06
Copy link
Contributor

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.

Pull Request Overview

This PR refactors environment configuration to allow optional environment variable usage for database and document paths while enforcing required API key validation. Key changes include:

  • Removing hard-coded environment variables from docker-compose.yml
  • Updating service calls in file_manager_service.py and document retrieval in app/routes/documents.py to use environment variables with fallback defaults
  • Adding an early environment variable check for OPENAI_API_KEY in app/config.py

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
docker-compose.yml Removed fixed environment definitions to allow dynamic configuration via env variables
app/services/file_manager_service.py Updated URL endpoints to be constructed based on DATABASE_URL from environment
app/routes/documents.py Modified document directory lookup using DOCUMENTS_DIR environment variable
app/config.py Added a check for OPENAI_API_KEY and default DATABASE_URL configuration

Comment on lines 129 to +130
upload_request_response = requests.post(
"http://database-api:8000/documents",
(os.getenv('DATABASE_URL') or "http://database-api:8000") + "/documents",
Copy link

Copilot AI May 15, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using a URL join function (e.g., urllib.parse.urljoin) to construct endpoints correctly and avoid potential issues with duplicated or missing slashes when DATABASE_URL already ends with a slash.

Copilot uses AI. Check for mistakes.
@CodeHex16 CodeHex16 deleted a comment from Copilot AI May 16, 2025
@lucaribon
Copy link
Member

le modifiche fatte portano nuovi problemi, probabilemente nelle chiamate fatte da llm alle api del db; in ogni caso non ha senso spendere tempo per sistemare questi dettagli e poi dover aggiornare documenti, soprattutto in questa fase del progetto.

@lucaribon lucaribon closed this May 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants