Add Docker support with Docker#23
Conversation
anilabhadatta
left a comment
anilabhadatta
left a comment
There was a problem hiding this comment.
I think you should revert the requirements.txt. New dependencies may cause issues with older python. Proper testing will be required.
There was a problem hiding this comment.
Pull request overview
This PR adds Docker containerization support to the Educative Viewer application, transitioning it from a package-based structure to a standalone Docker deployment with systemd integration for auto-start functionality.
Key changes:
- Added Docker support with Dockerfile, docker-compose.yml, and systemd service configuration
- Converted Python imports from relative (package-style) to absolute imports to support Docker deployment
- Updated README with comprehensive Docker installation and usage instructions
- Added environment variable support for configurable data storage location
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| Dockerfile | Defines Python 3.12-slim container with Flask application setup |
| docker-compose.yml | Orchestrates container with volume mounts and environment configuration |
| entrypoint.sh | Unused entrypoint script (appears to be remnant from earlier iteration) |
| educative-viewer.service | Systemd service file for auto-starting Docker container on boot |
| start-educative-viewer.sh | Installation script for systemd service setup with Tailscale integration |
| init.py | Changed imports from relative to absolute; added EDUCATIVE_VIEWER_ROOT env variable |
| auth.py | Changed imports from relative to absolute |
| db_utility.py | Changed imports from relative to absolute |
| main.py | Changed imports from relative to absolute; added EDUCATIVE_VIEWER_ROOT env variable |
| models.py | Changed imports from relative to absolute |
| pyproject.toml | Added Poetry configuration with pinned dependency versions |
| data/db.sqlite | Binary database file with user credentials (security concern) |
| command_to_run.txt | Sample environment variable export commands |
| cmd_export.txt | Sample environment variable export commands |
| README.md | Complete rewrite with Docker-first approach, service management, and configuration details |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docker-compose.yml
Outdated
| - downloadtoken=keystring | ||
| - EDUCATIVE_VIEWER_ROOT=/app/data | ||
| command: >- | ||
| python -c "from __init__ import create_app; app = create_app(); app.run(host='0.0.0.0', port=5000)" |
There was a problem hiding this comment.
Using Flask's development server in production is not recommended. The command app.run() starts Flask's built-in development server which is not designed for production use and lacks security, stability, and performance features. Since gunicorn is already included in the requirements, consider using it instead with a command like: gunicorn -w 4 -b 0.0.0.0:5000 '__init__:create_app()'
| python -c "from __init__ import create_app; app = create_app(); app.run(host='0.0.0.0', port=5000)" | |
| gunicorn -w 4 -b 0.0.0.0:5000 "__init__:create_app()" |
pyproject.toml
Outdated
| [tool.poetry] | ||
| name = "educative-viewer" | ||
| version = "0.1.0" | ||
| description = "" |
There was a problem hiding this comment.
Incomplete description field should be filled in. The description field is currently empty which makes the package metadata incomplete. Consider adding a brief description like "A web viewer for Educative.io courses" or similar.
| description = "" | |
| description = "A web viewer for Educative.io courses" |
pyproject.toml
Outdated
| name = "educative-viewer" | ||
| version = "0.1.0" | ||
| description = "" | ||
| authors = ["Your Name <you@example.com>"] |
There was a problem hiding this comment.
Placeholder author information should be replaced with actual details. The author field contains generic placeholder values "Your Name you@example.com" which should be updated with the actual author information or removed if not needed.
| authors = ["Your Name <you@example.com>"] |
| [Service] | ||
| Type=oneshot | ||
| RemainAfterExit=yes | ||
| WorkingDirectory=%h/Downloads/personal/educative/educative-viewer |
There was a problem hiding this comment.
Hardcoded user-specific path should be replaced with a placeholder or environment variable. Users will need to manually edit this path, which is error-prone. Consider using a variable like %h/educative-viewer or documenting that users need to update this path.
| WorkingDirectory=%h/Downloads/personal/educative/educative-viewer | |
| # Change this path if you cloned educative-viewer to a different location. | |
| WorkingDirectory=%h/educative-viewer |
| ports: | ||
| - "5000:5000" | ||
| volumes: | ||
| - /home/user/Downloads/personal/educative/courses:/course_data:ro,Z |
There was a problem hiding this comment.
Hardcoded user-specific path should be replaced with a placeholder. This path /home/user/Downloads/personal/educative/courses is specific to one user's system and will not work for others. Consider using a relative path like ./courses or adding a comment instructing users to modify this.
| - /home/user/Downloads/personal/educative/courses:/course_data:ro,Z | |
| - ./courses:/course_data:ro,Z |
Dockerfile
Outdated
| # Add the command to run the application | ||
| CMD ["python", "-c", "from __init__ import create_app; app = create_app(); app.run(host='0.0.0.0', port=5000)"] |
There was a problem hiding this comment.
Using Flask's development server in production is not recommended. The command app.run() starts Flask's built-in development server which is not designed for production use and lacks security, stability, and performance features. Since gunicorn is already included in requirements.txt, consider using it instead with a command like: gunicorn -w 4 -b 0.0.0.0:5000 '__init__:create_app()'
| # Add the command to run the application | |
| CMD ["python", "-c", "from __init__ import create_app; app = create_app(); app.run(host='0.0.0.0', port=5000)"] | |
| # Add the command to run the application using gunicorn (production-ready WSGI server) | |
| CMD ["gunicorn", "-w", "4", "-b", "0.0.0.0:5000", "__init__:create_app()"] |
docker-compose.yml
Outdated
| command: >- | ||
| python -c "from __init__ import create_app; app = create_app(); app.run(host='0.0.0.0', port=5000)" |
There was a problem hiding this comment.
The command in docker-compose.yml duplicates the CMD from the Dockerfile. When both are specified, the docker-compose command overrides the Dockerfile CMD. Consider removing the command from docker-compose.yml to use the Dockerfile's default, or remove the CMD from the Dockerfile if you prefer to define it here. Having both creates maintenance burden and confusion.
| command: >- | |
| python -c "from __init__ import create_app; app = create_app(); app.run(host='0.0.0.0', port=5000)" |
docker-compose.yml
Outdated
| - authtoken=keystring | ||
| - downloadtoken=keystring |
There was a problem hiding this comment.
Placeholder tokens should be replaced with more secure default values or removed. Having keystring as a default value may lead users to run the application with insecure credentials. Consider using empty strings or adding a validation check that prevents the application from starting with default/placeholder tokens.
| - authtoken=keystring | |
| - downloadtoken=keystring | |
| - authtoken= | |
| - downloadtoken= |
| from .models import CurrentPath, CourseDetails, User | ||
| from . import db | ||
| from models import CurrentPath, CourseDetails, User | ||
| from __init__ import db |
There was a problem hiding this comment.
Importing from __init__ module is an anti-pattern in Python. Instead of from __init__ import db, you should structure the imports differently. Common approaches include: 1) creating a separate extensions.py or database.py module to hold the db instance, or 2) using from app import db where app is the package name. Importing from __init__ directly can lead to circular import issues and makes the code less maintainable.
| from .models import User | ||
| from . import db | ||
| from models import User | ||
| from __init__ import db |
There was a problem hiding this comment.
Importing from __init__ module is an anti-pattern in Python. Instead of from __init__ import db, you should structure the imports differently. Common approaches include: 1) creating a separate extensions.py or database.py module to hold the db instance, or 2) using from app import db where app is the package name. Importing from __init__ directly can lead to circular import issues and makes the code less maintainable.
| from __init__ import db | |
| from . import db |
# This is the 1st commit message: # This is a combination of 12 commits. # This is the 1st commit message: Add Docker support with Dockerfile, docker-compose, and entrypoint script # This is the commit message anilabhadatta#2: update project setup files # This is the commit message anilabhadatta#3: SYNC FILES # This is the commit message anilabhadatta#4: Update docker-compose.yml # This is the commit message anilabhadatta#5: Update docker-compose.yml and add cmd_export.txt for environment configuration # This is the commit message anilabhadatta#6: Refactor Dockerfile and application structure for improved organization and environment configuration - Updated Dockerfile to copy application code and create data directory - Modified environment variable handling in __init__.py, auth.py, db_utility.py, main.py, and models.py - Adjusted docker-compose.yml for proper environment variable setup and command execution # This is the commit message anilabhadatta#7: Add systemd service and installer script for Educative Viewer with rootless Podman support # This is the commit message anilabhadatta#8: Refactor Dockerfile for improved readability and consistency in commands # This is the commit message anilabhadatta#9: Add ExecStartPre command to clean up existing Podman container before starting # This is the commit message anilabhadatta#10: Add Docker/Podman persistence instructions and initial database file # This is the commit message anilabhadatta#11: Update README and service installer for Docker support; remove Podman references # This is the commit message anilabhadatta#12: Refactor code structure for improved readability and maintainability # This is the commit message anilabhadatta#2: Refactor topic handling in main.py and improve utility functions in os_utility.py for better clarity and functionality # This is the commit message anilabhadatta#3: Use docker compose with Podman socket Set DOCKER_HOST to the user's Podman socket and remove any existing container before start; switch docker-compose calls to 'docker compose' # This is the commit message anilabhadatta#4: feat: enhance Docker build with improved dependency caching, user setup, and a healthcheck. # This is the commit message anilabhadatta#5: feat: Add a health check endpoint for container orchestration # This is the commit message anilabhadatta#6: Change service port from 5000 to 5001 # This is the commit message anilabhadatta#7: Add AzureRM provider and initial Terraform configuration - Added LICENSE file for the AzureRM provider. - Created README.md for the AzureRM provider with usage examples. - Added terraform-provider-azurerm binary file. - Implemented main.tf with resources for Azure Resource Group, Storage Account, Log Analytics Workspace, and Container App. - Created outputs.tf to define outputs for the deployed resources. - Added provider.tf to specify the AzureRM provider configuration. - Included terraform.tfvars.example for example variable values. - Defined variables in variables.tf for project configuration. - Updated lock file for dependency management. Signed-off-by: DeiAsPie <93835541+DeiAsPie@users.noreply.github.com>
Signed-off-by: DeiAsPie <93835541+DeiAsPie@users.noreply.github.com>
2 participants
No description provided.