Skip to content

Conversation

@emy3
Copy link
Owner

@emy3 emy3 commented Oct 11, 2025

📝 Description

Adding the initial project structure with UI folders and other folders

What does this PR do?

Structuring this project for better maintainability.

Why is this change necessary?

With more features and additions to the project, it will need to be structured better

🔄 Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🎨 Code style update (formatting, renaming)
  • ♻️ Code refactoring (no functional changes)
  • ⚡ Performance improvement
  • ✅ Test updates
  • 🔧 Build/CI configuration changes
  • 🗑️ Deprecation or removal of features

🧪 Testing

Test Environment

  • Python version:
  • Docker version:
  • OS:

How Has This Been Tested?

  • Tested with multiple running containers
  • Tested with no running containers
  • Tested keyboard interrupt (Ctrl+C)
  • Verified color-coded output works correctly
  • Tested on different operating systems
  • Manually tested the changes
  • Added/updated unit tests
  • All existing tests pass

Test Commands

# Commands used to test the changes
python dockerLog.py

📋 Checklist

  • My code follows the project's code style
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md)
  • My changes generate no new warnings or errors
  • I have tested my changes locally
  • Any dependent changes have been merged and published
  • I have updated the requirements if new dependencies were added

📸 Screenshots/Output

Before

<!-- Paste output or screenshot from before the changes -->

After

<!-- Paste output or screenshot from after the changes -->

🔗 Related Issues

Closes #
Related to #

📝 Additional Notes

🔍 Reviewer Notes


@emy3 emy3 requested a review from Copilot October 13, 2025 20:20
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.

Pull Request Overview

This PR establishes the initial project structure for a Docker logs monitoring application, creating organized folders and base UI components for better maintainability.

  • Introduces a modular structure with separate packages for widgets, screens, services, and utilities
  • Creates foundational UI components including header, footer, controls, and metrics widgets
  • Sets up placeholder files for Docker services and configuration utilities

Reviewed Changes

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

Show a summary per file
File Description
src/docker_logs/widgets/metrics_widget.py Implements real-time metrics display widget with reactive properties for thread count, container count, and monitoring status
src/docker_logs/widgets/header.py Creates custom header widget extending Textual's Header with app title and dynamic status display
src/docker_logs/widgets/footer.py Defines basic footer widget extending Textual's Footer for keybindings display
src/docker_logs/widgets/controls_widget.py Implements control panel with start/stop/refresh/clear buttons and corresponding message classes
src/docker_logs/widgets/init.py Package initialization file with widget imports and exports
src/docker_logs/utils/config.py Placeholder file for configuration utilities
src/docker_logs/utils/colors.py Color management utility for assigning random colors to Docker containers
src/docker_logs/services/docker_services.py Placeholder file for Docker-related services
src/docker_logs/screens/main_screen.py Placeholder file for the main log viewer screen
src/docker_logs/screens/init.py Package initialization file for screens module
src/docker_logs/dockerLog.py Updated main app file with documentation header
layout.py Example layout file demonstrating Textual app structure

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 3 to 10
from ..widgets import Header, Footer, MetricsWidget, ControlsWidget, LogViewerWidget

__all__ = [
"Header",
"Footer",
"MetricsWidget",
"ControlsWidget",
"LogViewerWidget"
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

The import statement references 'LogViewerWidget' which doesn't exist in any of the created files. This will cause an ImportError.

Suggested change
from ..widgets import Header, Footer, MetricsWidget, ControlsWidget, LogViewerWidget
__all__ = [
"Header",
"Footer",
"MetricsWidget",
"ControlsWidget",
"LogViewerWidget"
from ..widgets import Header, Footer, MetricsWidget, ControlsWidget
__all__ = [
"Header",
"Footer",
"MetricsWidget",
"ControlsWidget",

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +5
from .main_screen import MainScreen

__all__ = ["MainScreen"] No newline at end of file
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

The import statement references 'MainScreen' class which doesn't exist in main_screen.py. The file only contains an import of textual.app.App.

Suggested change
from .main_screen import MainScreen
__all__ = ["MainScreen"]
__all__ = []

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings October 18, 2025 17:50
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.

Pull Request Overview

Copilot reviewed 14 out of 19 changed files in this pull request and generated 9 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +3 to +7
from textual.widget import static
from textual.reactive import reactive
from threading import active_count

class MetricsWidget(static):
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

The import statement is incorrect. Textual's Static widget should be imported with a capital 'S': from textual.widgets import Static. The current lowercase 'static' will cause an ImportError.

Suggested change
from textual.widget import static
from textual.reactive import reactive
from threading import active_count
class MetricsWidget(static):
from textual.widgets import Static
from textual.reactive import reactive
from threading import active_count
class MetricsWidget(Static):

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +7
from textual.widget import static
from textual.reactive import reactive
from threading import active_count

class MetricsWidget(static):
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

The class inherits from 'static' (lowercase), but should inherit from 'Static' (capitalized). This is a consequence of the incorrect import on line 3 and will cause a NameError.

Suggested change
from textual.widget import static
from textual.reactive import reactive
from threading import active_count
class MetricsWidget(static):
from textual.widget import Static
from textual.reactive import reactive
from threading import active_count
class MetricsWidget(Static):

Copilot uses AI. Check for mistakes.
super().__init__(max_lines=max_lines)
self.max_lines = max_lines

def write_log(self, container_name: str, message: str, color:str) -> None:
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

Missing space after colon in the type annotation for 'color' parameter. Should be 'color: str' to conform to PEP 8 style guidelines.

Suggested change
def write_log(self, container_name: str, message: str, color:str) -> None:
def write_log(self, container_name: str, message: str, color: str) -> None:

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,21 @@
"""Log viewer widget."""

from textual.widget import RichLog
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

Incorrect import path for RichLog. The correct import should be from textual.widgets import RichLog (plural 'widgets', not singular 'widget').

Suggested change
from textual.widget import RichLog
from textual.widgets import RichLog

Copilot uses AI. Check for mistakes.
Comment on lines +66 to +76
class StartMonitoring:
pass

class StopMonitoring:
pass

class ClearLogs:
pass

class RefreshContainers:
pass No newline at end of file
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

These message classes don't inherit from textual.message.Message. They should inherit from Message to work properly with Textual's message handling system, similar to how they're defined in ControlsWidget.

Copilot uses AI. Check for mistakes.
from textual import on

from .screens.main_screen import MainScreen
from .services.docker_service import DockerService
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

Attempting to import DockerService from a file that only contains a docstring comment. The docker_service.py file needs to implement the DockerService class before this import will work.

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +13
from .utils.config import config

Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

Attempting to import config from a file that only contains a docstring comment. The config.py file needs to define the config object with attributes like 'default_colors' and 'update_interval' before this import will work.

Suggested change
from .utils.config import config
try:
from .utils.config import config
except ImportError:
from types import SimpleNamespace
config = SimpleNamespace(
default_colors=["red", "green", "blue", "yellow", "magenta", "cyan"],
update_interval=1.0
)

Copilot uses AI. Check for mistakes.
container_count = self.docker_service.get_container_count()
monitoring = self.docker_service.is_monitoring()

self.main_screen.refresh_metrics(container_count, monitoring)
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

The MainScreen class doesn't have a 'refresh_metrics' method. Based on the structure, this should likely call 'self.main_screen.metrics.refresh_metrics(container_count, monitoring)'.

Suggested change
self.main_screen.refresh_metrics(container_count, monitoring)
self.main_screen.metrics.refresh_metrics(container_count, monitoring)

Copilot uses AI. Check for mistakes.
def action_clear_logs(self) -> None:
self.main_screen.log_viewer.clear_logs()
logger.info("Cleared logs")

Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

The method calls 'self.handle_refresh_containers()' which is not defined in this class. The method should either be implemented or this should post a RefreshContainers message.

Suggested change
def handle_refresh_containers(self) -> None:
"""Refresh the list of containers and update the UI."""
containers = self.docker_service.get_containers()
self.main_screen.refresh_container_list(containers)

Copilot uses AI. Check for mistakes.
@emy3 emy3 requested a review from Copilot October 20, 2025 10:02
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.

Pull Request Overview

Copilot reviewed 14 out of 19 changed files in this pull request and generated 10 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


__all__ = [
"Header",
"Footer",
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Trailing whitespace after 'Footer', - this line has inconsistent whitespace compared to other entries in the list.

Suggested change
"Footer",
"Footer",

Copilot uses AI. Check for mistakes.
from .screens.main_screen import MainScreen
from .services.docker_service import DockerService
from .utils.colors import ColorManager
from .utils.config import config
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Importing config from config.py, but that file only contains a docstring with no config object defined. This will result in an ImportError.

Copilot uses AI. Check for mistakes.
def __init__(self):
super().__init__()
self.docker_service = DockerService()
self.color_manager = ColorManager(config.default_colors)
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Attempting to access config.default_colors, but config is not defined in utils/config.py. This will cause an AttributeError at runtime.

Copilot uses AI. Check for mistakes.
def on_mount(self) -> None:
"""Called when the app starts"""
self.push_screen(self.main_screen)
self.set_interval(config.update_interval, self._update_metrics)
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Attempting to access config.update_interval, but config is not defined in utils/config.py. This will cause an AttributeError at runtime.

Copilot uses AI. Check for mistakes.
Comment on lines +45 to +46
container_count = self.docker_service.get_container_count()
monitoring = self.docker_service.is_monitoring()
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Calling methods on docker_service, but DockerService class is not defined. These method calls will fail at runtime.

Copilot uses AI. Check for mistakes.
container_count = self.docker_service.get_container_count()
monitoring = self.docker_service.is_monitoring()

self.main_screen.refresh_metrics(container_count, monitoring)
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Calling refresh_metrics on main_screen, but MainScreen class has no such method defined. This will result in an AttributeError.

Copilot uses AI. Check for mistakes.

def _process_logs(self) -> None:
""" Process the incoming log messages"""
for container_name, message in self.docker_service.get_log_messages():
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Calling get_log_messages on docker_service, but this method is not defined in the DockerService class. This will fail at runtime.

Copilot uses AI. Check for mistakes.
# Setup event handlers
@on(MainScreen.StartMonitoring)
def handle_start_monitoring(self) -> None:
if self.docker_service.start_monitoring():
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Calling start_monitoring on docker_service, but this method is not defined. This will fail at runtime.

Copilot uses AI. Check for mistakes.

@on(MainScreen.StopMonitoring)
def handle_stop_monitoring(self) -> None:
self.docker_service.stop_monitoring()
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Calling stop_monitoring on docker_service, but this method is not defined. This will fail at runtime.

Copilot uses AI. Check for mistakes.
Comment on lines +79 to +80
def action_refresh(self) -> None:
self.handle_refresh_containers()
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Method handle_refresh_containers is called but not defined in the DockerLogsApp class. This will result in an AttributeError.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants