Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 21, 2025

The Docker deployment documentation and configuration files contained multiple inconsistencies that caused validation errors during container startup. This fix addresses both the original HOST_URL vs AGENT_HOST_URL mismatch and additional configuration documentation issues.

Issues Fixed:

  1. Environment variable name mismatch: Docker command showed HOST_URL but application expects AGENT_HOST_URL
  2. Inconsistent .env.example: Still used deprecated HOST_URL variable name
  3. Misleading documentation: DATA_FOLDER_PATH was listed as "optional" when it's actually required when S3 is disabled

Before (broken configuration):

# Docker command used wrong variable name
docker run -e HOST_URL=http://localhost:4322 ...

# .env.example showed incorrect variable
HOST_URL=http://localhost:4322

# Documentation incorrectly listed DATA_FOLDER_PATH as optional
**Optional Environment Variables:**
- `DATA_FOLDER_PATH`: Local storage path (default: `data`)

This caused validation errors:

ValidationError: 2 validation errors for AppConfig
agent_host_url
  Value error, agent_host_url environment variable is required.
data_folder_path
  Value error, Data folder path must be set when S3 is not enabled.

After (working configuration):

# Correct variable names throughout
docker run -e AGENT_HOST_URL=http://localhost:4322 ...
AGENT_HOST_URL=http://localhost:4322

# Clear storage configuration requirements
**Storage Configuration (choose one):**
When S3 is disabled (default), local storage is required:
- `DATA_FOLDER_PATH`: Local storage path for allocation data (required when `S3_ENABLED=false`)

Changes Made:

  • Updated Docker command to use AGENT_HOST_URL instead of HOST_URL
  • Fixed .env.example to use correct environment variable names
  • Reorganized documentation to clearly explain storage configuration requirements
  • Clarified that DATA_FOLDER_PATH is required when S3 is disabled (default behavior)

The corrected documentation now matches the actual validation logic in allocator_bot/models.py and is consistent across all configuration files including Kubernetes manifests.

Fixes #4.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Aug 21, 2025
Copy link
Contributor Author

Copilot AI commented Aug 21, 2025

@piiq 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

Copilot AI changed the title [WIP] Docker cannot be deployed Fix Docker deployment environment variable names in README.md Aug 21, 2025
Copilot AI requested a review from piiq August 21, 2025 19:52
@piiq
Copy link
Owner

piiq commented Aug 22, 2025

@copilot check that the logic and the documentation around data_folder_path is correct. The

data_folder_path
  Value error, Data folder path must be set when S3 is not enabled.

error doesn't seem related to the host url.

Copy link
Contributor Author

Copilot AI commented Aug 22, 2025

@copilot check that the logic and the documentation around data_folder_path is correct. The

data_folder_path...

You're absolutely correct! The data_folder_path error was unrelated to the host URL fix. I found and corrected several inconsistencies:

Issues found:

  1. .env.example still had HOST_URL= instead of AGENT_HOST_URL=
  2. README.md incorrectly listed DATA_FOLDER_PATH as "optional" when it's actually required when S3 is disabled
  3. Documentation didn't clearly explain the storage configuration requirements

Fixed in commit 2526bd8:

  • Updated .env.example to use correct AGENT_HOST_URL variable name
  • Reorganized README.md environment variables section to clarify that DATA_FOLDER_PATH is required when S3_ENABLED=false (the default)
  • Added clear storage configuration documentation explaining the either/or relationship between local and S3 storage

The validation logic in models.py was correct - it properly enforces that data_folder_path must be set when S3 is not enabled. The issue was that the documentation didn't match this validation requirement.

Copilot AI changed the title Fix Docker deployment environment variable names in README.md Fix Docker deployment and configuration documentation inconsistencies Aug 22, 2025
@piiq piiq marked this pull request as ready for review August 22, 2025 12:42
@piiq piiq merged commit 6a2a49d into main Aug 22, 2025
1 check passed
@piiq piiq deleted the copilot/fix-4 branch August 22, 2025 12:43
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.

Docker cannot be deployed

2 participants