DRAFT: v0.1.0 #1020
Open
DRAFT: v0.1.0 #1020
+41,000
−28,253
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deploying tux with
|
| Latest commit: |
5c2f948
|
| Status: | 🚫 Build failed. |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found. |
|
@sourcery-ai review |
Reviewer's GuideThis PR modernizes the Tux codebase by reorganizing source files under a Entity relationship diagram for database controller module reorganizationerDiagram
AfkController ||--o{ DatabaseController : provides
CaseController ||--o{ DatabaseController : provides
GuildController ||--o{ DatabaseController : provides
GuildConfigController ||--o{ DatabaseController : provides
LevelsController ||--o{ DatabaseController : provides
NoteController ||--o{ DatabaseController : provides
ReminderController ||--o{ DatabaseController : provides
SnippetController ||--o{ DatabaseController : provides
StarboardController ||--o{ DatabaseController : provides
StarboardMessageController ||--o{ DatabaseController : provides
DatabaseController }o--|| IDatabaseService : implements
Class diagram for Tux bot and dependency injection changesclassDiagram
class Tux {
+ServiceContainer container
+SentryManager sentry_manager
+EmojiManager emoji_manager
+DatabaseService db
+setup()
+_setup_container()
+_validate_container()
+_raise_container_validation_error()
+_cleanup_container()
}
class ServiceContainer {
+get_optional(interface)
}
class ServiceRegistry {
+configure_container(bot)
+validate_container(container)
+get_registered_services(container)
}
class SentryManager {
+setup()
+capture_exception()
+finish_transaction_on_error()
+set_command_context()
+flush_async()
+is_initialized
}
Tux --> ServiceContainer
Tux --> SentryManager
Tux --> EmojiManager
Tux --> DatabaseService
ServiceRegistry --> ServiceContainer
ServiceContainer --> SentryManager
ServiceContainer --> EmojiManager
ServiceContainer --> DatabaseService
Class diagram for ConfigSet* UI views with DI changesclassDiagram
class ConfigSetPrivateLogs {
+__init__(timeout, bot, db_service)
-db: GuildConfigController
}
class ConfigSetPublicLogs {
+__init__(timeout, bot, db_service)
-db: GuildConfigController
}
class ConfigSetChannels {
+__init__(timeout, bot, db_service)
-db: GuildConfigController
}
class IDatabaseService {
+get_controller()
}
class GuildConfigController {
}
ConfigSetPrivateLogs --> IDatabaseService
ConfigSetPublicLogs --> IDatabaseService
ConfigSetChannels --> IDatabaseService
IDatabaseService --> GuildConfigController
Class diagram for Setup cog refactor to use DI and base classclassDiagram
class Setup {
+__init__(bot)
-config: GuildConfigController
}
class BaseCog {
+__init__(bot)
-db: IDatabaseService
}
class IDatabaseService {
+get_controller()
}
class GuildConfigController {
}
Setup --|> BaseCog
BaseCog --> IDatabaseService
IDatabaseService --> GuildConfigController
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @kzndotsh - I've reviewed your changes and they look great!
Blocking issues:
- By not specifying a USER, a program in the container may run as 'root'. This is a security hazard. If an attacker can control a process running as root, they may have control over the container. Ensure that the last USER in a Dockerfile is a USER other than 'root'. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/tux/services/tracing.py:80` </location>
<code_context>
+# --- Common Helpers ---
+
+
+def safe_set_name(obj: Any, name: str) -> None:
+ """
+ Safely set the name on a span or transaction object.
</code_context>
<issue_to_address>
safe_set_name may fail silently if set_name is not callable.
Add a check to ensure set_name is callable before invoking it to prevent silent failures.
</issue_to_address>
### Comment 2
<location> `src/tux/services/tracing.py:598` </location>
<code_context>
+ raise
+
+
+def instrument_bot_commands(bot: commands.Bot) -> None:
+ """
+ Automatically instruments all bot commands with Sentry transactions.
</code_context>
<issue_to_address>
Instrumenting bot commands by replacing callbacks may interfere with other decorators.
Directly replacing the command callback can disrupt existing decorators or attributes. Using a wrapper or middleware would help maintain original metadata.
</issue_to_address>
### Comment 3
<location> `src/tux/core/container.py:244` </location>
<code_context>
+ return descriptor.instance
+
+ # Create new instance
+ self._resolution_stack.add(service_type)
+
+ try:
</code_context>
<issue_to_address>
Using a set for _resolution_stack may not preserve dependency order for error reporting.
Consider replacing the set with a list or stack to maintain resolution order, which will improve error messages for circular dependencies.
Suggested implementation:
```python
# Create new instance
self._resolution_stack.append(service_type)
```
```python
try:
```
```python
except Exception as e:
stack_trace = " -> ".join([t.__name__ for t in self._resolution_stack])
logger.error(f"Failed to resolve {service_type.__name__}: {e}\nResolution stack: {stack_trace}")
error_msg = f"Cannot resolve {service_type.__name__} (resolution stack: {stack_trace})"
raise ServiceResolutionError(error_msg) from e
```
```python
else:
resolution_time = time.perf_counter() - start_time
# Only log if resolution takes longer than expected or fails
if resolution_time > 0.001: # Log if takes more than 1ms
logger.debug(f"Slow resolution: {service_type.__name__} took {resolution_time:.4f}s")
self._resolution_stack.pop()
```
You will also need to:
1. Change the initialization of `self._resolution_stack` from a set to a list (e.g., `self._resolution_stack = []`).
2. Update any other code that checks membership (`in`) or removes items from `_resolution_stack` to use list semantics.
3. If you have circular dependency checks, you can still use `if service_type in self._resolution_stack:` for lists.
</issue_to_address>
### Comment 4
<location> `src/tux/core/container.py:299` </location>
<code_context>
+ # Resolve the dependency
+ dependency = self._resolve_service(param_type)
+
+ if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD:
+ if param.default is inspect.Parameter.empty:
+ args.append(dependency)
+ else:
+ kwargs[param.name] = dependency
+ elif param.kind == inspect.Parameter.KEYWORD_ONLY:
+ kwargs[param.name] = dependency
+
</code_context>
<issue_to_address>
Does not handle VAR_POSITIONAL or VAR_KEYWORD constructor parameters.
Currently, *args and **kwargs in service constructors are not processed. Please add explicit handling for VAR_POSITIONAL and VAR_KEYWORD parameters, either by raising an error or documenting their unsupported status.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
# Resolve the dependency
dependency = self._resolve_service(param_type)
if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD:
if param.default is inspect.Parameter.empty:
args.append(dependency)
else:
kwargs[param.name] = dependency
elif param.kind == inspect.Parameter.KEYWORD_ONLY:
kwargs[param.name] = dependency
=======
# Resolve the dependency
dependency = self._resolve_service(param_type)
if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD:
if param.default is inspect.Parameter.empty:
args.append(dependency)
else:
kwargs[param.name] = dependency
elif param.kind == inspect.Parameter.KEYWORD_ONLY:
kwargs[param.name] = dependency
elif param.kind == inspect.Parameter.VAR_POSITIONAL:
raise ServiceResolutionError(
f"Constructor parameter '*{param.name}' in {impl_type.__name__} is not supported by the DI container"
)
elif param.kind == inspect.Parameter.VAR_KEYWORD:
raise ServiceResolutionError(
f"Constructor parameter '**{param.name}' in {impl_type.__name__} is not supported by the DI container"
)
>>>>>>> REPLACE
</suggested_fix>
### Comment 5
<location> `docs/content/dev/local_development.md:37` </location>
<code_context>
The project includes a hot-reloading utility (`tux/utils/hot_reload.py`).
-When the bot is running locally via `poetry run tux --dev start`, this utility watches for changes in the `tux/cogs/` directory. It attempts to automatically reload modified cogs or cogs affected by changes in watched utility files without requiring a full bot restart.
+When the bot is running locally via `uv run tux --dev start`, this utility watches for changes in the `tux/cogs/` directory. It attempts to automatically reload modified cogs or cogs affected by changes in watched utility files without requiring a full bot restart.
-This significantly speeds up development for cog-related changes. Note that changes outside the watched directories (e.g., core bot logic, dependencies) may still require a manual restart (`Ctrl+C` and run the start command again).
</code_context>
<issue_to_address>
Update directory and terminology to match new module structure.
Please update all directory and terminology references to align with the new module structure for consistency.
</issue_to_address>
## Security Issues
### Issue 1
<location> `Dockerfile:259` </location>
<issue_to_address>
**security (dockerfile.security.missing-user):** By not specifying a USER, a program in the container may run as 'root'. This is a security hazard. If an attacker can control a process running as root, they may have control over the container. Ensure that the last USER in a Dockerfile is a USER other than 'root'.
```suggestion
USER non-root
CMD ["sh", "-c", "uv run prisma generate && exec uv run tux --dev start"]
```
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # --- Common Helpers --- | ||
|
|
||
|
|
||
| def safe_set_name(obj: Any, name: str) -> None: |
There was a problem hiding this comment.
suggestion (bug_risk): safe_set_name may fail silently if set_name is not callable.
Add a check to ensure set_name is callable before invoking it to prevent silent failures.
| raise | ||
|
|
||
|
|
||
| def instrument_bot_commands(bot: commands.Bot) -> None: |
There was a problem hiding this comment.
issue: Instrumenting bot commands by replacing callbacks may interfere with other decorators.
Directly replacing the command callback can disrupt existing decorators or attributes. Using a wrapper or middleware would help maintain original metadata.
src/tux/core/container.py
Outdated
| return descriptor.instance | ||
|
|
||
| # Create new instance | ||
| self._resolution_stack.add(service_type) |
There was a problem hiding this comment.
suggestion: Using a set for _resolution_stack may not preserve dependency order for error reporting.
Consider replacing the set with a list or stack to maintain resolution order, which will improve error messages for circular dependencies.
Suggested implementation:
# Create new instance
self._resolution_stack.append(service_type) try: except Exception as e:
stack_trace = " -> ".join([t.__name__ for t in self._resolution_stack])
logger.error(f"Failed to resolve {service_type.__name__}: {e}\nResolution stack: {stack_trace}")
error_msg = f"Cannot resolve {service_type.__name__} (resolution stack: {stack_trace})"
raise ServiceResolutionError(error_msg) from e else:
resolution_time = time.perf_counter() - start_time
# Only log if resolution takes longer than expected or fails
if resolution_time > 0.001: # Log if takes more than 1ms
logger.debug(f"Slow resolution: {service_type.__name__} took {resolution_time:.4f}s")
self._resolution_stack.pop()You will also need to:
- Change the initialization of
self._resolution_stackfrom a set to a list (e.g.,self._resolution_stack = []). - Update any other code that checks membership (
in) or removes items from_resolution_stackto use list semantics. - If you have circular dependency checks, you can still use
if service_type in self._resolution_stack:for lists.
src/tux/core/container.py
Outdated
Comment on lines
296
to
305
| # Resolve the dependency | ||
| dependency = self._resolve_service(param_type) | ||
|
|
||
| if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD: | ||
| if param.default is inspect.Parameter.empty: | ||
| args.append(dependency) | ||
| else: | ||
| kwargs[param.name] = dependency | ||
| elif param.kind == inspect.Parameter.KEYWORD_ONLY: | ||
| kwargs[param.name] = dependency |
There was a problem hiding this comment.
suggestion: Does not handle VAR_POSITIONAL or VAR_KEYWORD constructor parameters.
Currently, *args and **kwargs in service constructors are not processed. Please add explicit handling for VAR_POSITIONAL and VAR_KEYWORD parameters, either by raising an error or documenting their unsupported status.
Suggested change
| # Resolve the dependency | |
| dependency = self._resolve_service(param_type) | |
| if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD: | |
| if param.default is inspect.Parameter.empty: | |
| args.append(dependency) | |
| else: | |
| kwargs[param.name] = dependency | |
| elif param.kind == inspect.Parameter.KEYWORD_ONLY: | |
| kwargs[param.name] = dependency | |
| # Resolve the dependency | |
| dependency = self._resolve_service(param_type) | |
| if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD: | |
| if param.default is inspect.Parameter.empty: | |
| args.append(dependency) | |
| else: | |
| kwargs[param.name] = dependency | |
| elif param.kind == inspect.Parameter.KEYWORD_ONLY: | |
| kwargs[param.name] = dependency | |
| elif param.kind == inspect.Parameter.VAR_POSITIONAL: | |
| raise ServiceResolutionError( | |
| f"Constructor parameter '*{param.name}' in {impl_type.__name__} is not supported by the DI container" | |
| ) | |
| elif param.kind == inspect.Parameter.VAR_KEYWORD: | |
| raise ServiceResolutionError( | |
| f"Constructor parameter '**{param.name}' in {impl_type.__name__} is not supported by the DI container" | |
| ) |
| The project includes a hot-reloading utility (`tux/utils/hot_reload.py`). | ||
|
|
||
| When the bot is running locally via `poetry run tux --dev start`, this utility watches for changes in the `tux/cogs/` directory. It attempts to automatically reload modified cogs or cogs affected by changes in watched utility files without requiring a full bot restart. | ||
| When the bot is running locally via `uv run tux --dev start`, this utility watches for changes in the `tux/cogs/` directory. It attempts to automatically reload modified cogs or cogs affected by changes in watched utility files without requiring a full bot restart. |
There was a problem hiding this comment.
suggestion: Update directory and terminology to match new module structure.
Please update all directory and terminology references to align with the new module structure for consistency.
Dockerfile
Outdated
| # WORKFLOW: Regenerates Prisma client and starts the bot in development mode | ||
| # This ensures the database client is always up-to-date with schema changes | ||
| CMD ["sh", "-c", "poetry run prisma generate && exec poetry run tux --dev start"] | ||
| CMD ["sh", "-c", "uv run prisma generate && exec uv run tux --dev start"] |
There was a problem hiding this comment.
security (dockerfile.security.missing-user): By not specifying a USER, a program in the container may run as 'root'. This is a security hazard. If an attacker can control a process running as root, they may have control over the container. Ensure that the last USER in a Dockerfile is a USER other than 'root'.
Suggested change
| CMD ["sh", "-c", "uv run prisma generate && exec uv run tux --dev start"] | |
| USER non-root | |
| CMD ["sh", "-c", "uv run prisma generate && exec uv run tux --dev start"] |
Source: opengrep
tests/e2e/test_smoke_e2e.py
Outdated
Comment on lines
5
to
6
| # Keep E2E minimal and deterministic; expand with CLI or HTTP flows later | ||
| assert True |
There was a problem hiding this comment.
suggestion (code-quality): Remove assert True statements (remove-assert-true)
Suggested change
| # Keep E2E minimal and deterministic; expand with CLI or HTTP flows later | |
| assert True | |
| pass |
Comment on lines
5
to
6
| # Example of an integration placeholder; expand with real IO later | ||
| assert 1 + 1 == 2 |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Remove
assert Truestatements (remove-assert-true) - Simplify numeric comparison (
simplify-numeric-comparison) - Simplify x == x -> True and x != x -> False (
equality-identity)
Suggested change
| # Example of an integration placeholder; expand with real IO later | |
| assert 1 + 1 == 2 | |
| pass |
tests/unit/test_smoke.py
Outdated
|
|
||
| @pytest.mark.unit | ||
| def test_smoke() -> None: | ||
| assert True |
There was a problem hiding this comment.
suggestion (code-quality): Remove assert True statements (remove-assert-true)
Suggested change
| assert True | |
| pass |
There was a problem hiding this comment.
Hey @kzndotsh - I've reviewed your changes and they look great!
Blocking issues:
- By not specifying a USER, a program in the container may run as 'root'. This is a security hazard. If an attacker can control a process running as root, they may have control over the container. Ensure that the last USER in a Dockerfile is a USER other than 'root'. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/tux/services/sentry_manager.py:582` </location>
<code_context>
+ for key, value in tags.items():
+ scope.set_tag(key, value)
+
+ def set_user_context(self, user: discord.User | discord.Member) -> None:
+ """
+ Sets the user context for the current Sentry scope.
</code_context>
<issue_to_address>
set_user_context may expose sensitive user information.
Review the user and member fields being attached to Sentry to ensure only essential, non-sensitive information is included, particularly for production use.
</issue_to_address>
### Comment 2
<location> `src/tux/core/container.py:57` </location>
<code_context>
+ """Initialize an empty service container."""
+ self._services: dict[type, ServiceDescriptor] = {}
+ self._singleton_instances: dict[type, Any] = {}
+ self._resolution_stack: set[type] = set()
+
+ def register_singleton(self, service_type: type[T], implementation: type[T] | None = None) -> "ServiceContainer":
</code_context>
<issue_to_address>
Using a set for the resolution stack may obscure the actual dependency chain in circular dependency errors.
Using a list for the resolution stack will preserve the order of dependencies, making circular dependency errors easier to trace.
Suggested implementation:
```python
self._resolution_stack: list[type] = []
```
You will need to update all usages of `_resolution_stack` throughout the file:
- Replace `.add(x)` with `.append(x)`
- Replace `.remove(x)` with `.pop()` (if popping the last item) or `del` (if removing by index)
- If checking for membership, `x in self._resolution_stack` still works
- When reporting circular dependencies, you can now print the stack in order for better error messages
</issue_to_address>
### Comment 3
<location> `src/tux/core/container.py:235` </location>
<code_context>
+ descriptor = self._services[service_type]
+
+ # Return existing instance for singletons
+ if descriptor.lifetime == ServiceLifetime.SINGLETON:
+ if service_type in self._singleton_instances:
+ return self._singleton_instances[service_type]
+
+ # If we have a pre-registered instance, return it
+ if descriptor.instance is not None:
+ return descriptor.instance
+
</code_context>
<issue_to_address>
Singleton instance caching logic may be redundant if both instance and _singleton_instances are used.
Consolidate singleton instance handling to ensure a single source of truth and prevent precedence ambiguity.
Suggested implementation:
```python
# Return existing instance for singletons
if descriptor.lifetime == ServiceLifetime.SINGLETON:
if service_type in self._singleton_instances:
return self._singleton_instances[service_type]
```
To fully implement this, you must ensure that any pre-registered singleton instance (previously stored in `descriptor.instance`) is instead stored in `_singleton_instances` at registration time. This may require updating your service registration logic elsewhere in the codebase to move any instance assignment to `_singleton_instances`.
</issue_to_address>
### Comment 4
<location> `src/tux/core/context.py:95` </location>
<code_context>
+ A dictionary with standardized context keys like `user_id`,
+ `command_name`, `guild_id`, `command_type`, etc.
+ """
+ user = source.user if isinstance(source, Interaction) else source.author
+
+ # Base context is common to both types
</code_context>
<issue_to_address>
Assumes that both Context and Interaction have user/author attributes, which may not always be true.
Add error handling or a fallback to prevent AttributeError if the expected attribute is missing.
</issue_to_address>
## Security Issues
### Issue 1
<location> `Dockerfile:259` </location>
<issue_to_address>
**security (dockerfile.security.missing-user):** By not specifying a USER, a program in the container may run as 'root'. This is a security hazard. If an attacker can control a process running as root, they may have control over the container. Ensure that the last USER in a Dockerfile is a USER other than 'root'.
```suggestion
USER non-root
CMD ["sh", "-c", "uv run prisma generate && exec uv run tux --dev start"]
```
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/tux/services/sentry_manager.py
Outdated
| for key, value in tags.items(): | ||
| scope.set_tag(key, value) | ||
|
|
||
| def set_user_context(self, user: discord.User | discord.Member) -> None: |
There was a problem hiding this comment.
🚨 suggestion (security): set_user_context may expose sensitive user information.
Review the user and member fields being attached to Sentry to ensure only essential, non-sensitive information is included, particularly for production use.
src/tux/core/container.py
Outdated
| """Initialize an empty service container.""" | ||
| self._services: dict[type, ServiceDescriptor] = {} | ||
| self._singleton_instances: dict[type, Any] = {} | ||
| self._resolution_stack: set[type] = set() |
There was a problem hiding this comment.
suggestion: Using a set for the resolution stack may obscure the actual dependency chain in circular dependency errors.
Using a list for the resolution stack will preserve the order of dependencies, making circular dependency errors easier to trace.
Suggested implementation:
self._resolution_stack: list[type] = []You will need to update all usages of _resolution_stack throughout the file:
- Replace
.add(x)with.append(x) - Replace
.remove(x)with.pop()(if popping the last item) ordel(if removing by index) - If checking for membership,
x in self._resolution_stackstill works - When reporting circular dependencies, you can now print the stack in order for better error messages
src/tux/core/container.py
Outdated
Comment on lines
235
to
240
| if descriptor.lifetime == ServiceLifetime.SINGLETON: | ||
| if service_type in self._singleton_instances: | ||
| return self._singleton_instances[service_type] | ||
|
|
||
| # If we have a pre-registered instance, return it | ||
| if descriptor.instance is not None: |
There was a problem hiding this comment.
suggestion: Singleton instance caching logic may be redundant if both instance and _singleton_instances are used.
Consolidate singleton instance handling to ensure a single source of truth and prevent precedence ambiguity.
Suggested implementation:
# Return existing instance for singletons
if descriptor.lifetime == ServiceLifetime.SINGLETON:
if service_type in self._singleton_instances:
return self._singleton_instances[service_type]To fully implement this, you must ensure that any pre-registered singleton instance (previously stored in descriptor.instance) is instead stored in _singleton_instances at registration time. This may require updating your service registration logic elsewhere in the codebase to move any instance assignment to _singleton_instances.
src/tux/core/context.py
Outdated
| A dictionary with standardized context keys like `user_id`, | ||
| `command_name`, `guild_id`, `command_type`, etc. | ||
| """ | ||
| user = source.user if isinstance(source, Interaction) else source.author |
There was a problem hiding this comment.
issue: Assumes that both Context and Interaction have user/author attributes, which may not always be true.
Add error handling or a fallback to prevent AttributeError if the expected attribute is missing.
Dockerfile
Outdated
| # WORKFLOW: Regenerates Prisma client and starts the bot in development mode | ||
| # This ensures the database client is always up-to-date with schema changes | ||
| CMD ["sh", "-c", "poetry run prisma generate && exec poetry run tux --dev start"] | ||
| CMD ["sh", "-c", "uv run prisma generate && exec uv run tux --dev start"] |
There was a problem hiding this comment.
security (dockerfile.security.missing-user): By not specifying a USER, a program in the container may run as 'root'. This is a security hazard. If an attacker can control a process running as root, they may have control over the container. Ensure that the last USER in a Dockerfile is a USER other than 'root'.
Suggested change
| CMD ["sh", "-c", "uv run prisma generate && exec uv run tux --dev start"] | |
| USER non-root | |
| CMD ["sh", "-c", "uv run prisma generate && exec uv run tux --dev start"] |
Source: opengrep
tests/e2e/test_smoke_e2e.py
Outdated
Comment on lines
5
to
6
| # Keep E2E minimal and deterministic; expand with CLI or HTTP flows later | ||
| assert True |
There was a problem hiding this comment.
suggestion (code-quality): Remove assert True statements (remove-assert-true)
Suggested change
| # Keep E2E minimal and deterministic; expand with CLI or HTTP flows later | |
| assert True | |
| pass |
Comment on lines
5
to
6
| # Example of an integration placeholder; expand with real IO later | ||
| assert 1 + 1 == 2 |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Remove
assert Truestatements (remove-assert-true) - Simplify numeric comparison (
simplify-numeric-comparison) - Simplify x == x -> True and x != x -> False (
equality-identity)
Suggested change
| # Example of an integration placeholder; expand with real IO later | |
| assert 1 + 1 == 2 | |
| pass |
tests/unit/test_smoke.py
Outdated
|
|
||
| @pytest.mark.unit | ||
| def test_smoke() -> None: | ||
| assert True |
There was a problem hiding this comment.
suggestion (code-quality): Remove assert True statements (remove-assert-true)
Suggested change
| assert True | |
| pass |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: admin <admin@kaizen.wtf>
Co-authored-by: admin <admin@kaizen.wtf>
Co-authored-by: admin <admin@kaizen.wtf>
… modules Co-authored-by: admin <admin@kaizen.wtf>
Co-authored-by: admin <admin@kaizen.wtf>
…gging Co-authored-by: admin <admin@kaizen.wtf>
Co-authored-by: admin <admin@kaizen.wtf>
Co-authored-by: admin <admin@kaizen.wtf>
Co-authored-by: admin <admin@kaizen.wtf>
Co-authored-by: admin <admin@kaizen.wtf>
…models Co-authored-by: admin <admin@kaizen.wtf>
Co-authored-by: admin <admin@kaizen.wtf>
Co-authored-by: admin <admin@kaizen.wtf>
Co-authored-by: admin <admin@kaizen.wtf>
…tion - Introduced new fixtures for database management, including `pglite_async_manager`, `db_service`, and controllers for guild configurations. - Added Sentry-related mock fixtures to facilitate testing of Discord interactions and error handling. - Updated the test fixtures package to import all new fixtures for easier access during testing. - Removed unnecessary cleanup functionality from PGlite fixtures due to upstream library improvements.
- Replaced instances of specific database service fixtures with a unified `db_service` parameter across multiple integration test files for consistency. - Updated test methods to utilize the new parameter, enhancing readability and maintainability of the test code. - Introduced a new integration test file for database error handling, covering various scenarios including connection errors and transaction rollbacks. - Improved error handling tests to ensure robust coverage of database interactions and Sentry integration.
- Introduced comprehensive unit tests for the ErrorHandler and ErrorHandlerMixin classes, covering various error scenarios and ensuring proper logging and context setting. - Added tests for Sentry performance tracking, including command start and end tracking, to validate successful and failed command executions. - Implemented tests for Sentry service functions, ensuring robust error capturing and context management for Discord interactions. - Enhanced overall test coverage for error handling mechanisms, improving reliability and maintainability of the codebase.
- Introduced a new test file for end-to-end integration tests focused on the error handling flow within the Discord bot. - Implemented tests to verify user responses for command errors and app command errors, ensuring proper error handling and messaging. - Enhanced test coverage for the ErrorHandler class, validating its functionality in various error scenarios.
- Reduced the complexity of the conftest.py file by importing all fixtures from the fixtures directory. - Removed extensive cleanup and monitoring functions for PGlite processes, streamlining the test setup. - Focused on a minimalistic approach to enhance maintainability and clarity in test configurations.
- Eliminated the `capture_database_error` calls from both `AsyncDatabaseService` and `SyncDatabaseService` connection methods to streamline error handling. - Focused on improving clarity in error logging without external dependencies for database connection errors.
…modularization - Simplified the `setup` method by extracting setup steps into a new `_run_setup_steps` method for better organization and clarity. - Improved error handling for database connection and migration failures, providing clearer logging messages and reducing redundancy. - Streamlined the setup process by maintaining tracing and status tagging throughout the execution of setup steps.
- Introduced a new Role Count plugin specifically designed for the All Things Linux Discord server, featuring hardcoded role IDs. - Implemented functionality to display the number of users in various roles, categorized by distribution, language, desktop environment/window manager, editors, and vanity roles. - Added command handling and embed creation for user-friendly interaction responses. - Emphasized that this plugin is server-specific and should not be used on other Discord servers.
…reation - Added try-except blocks to improve error handling in the `Config` and `Setup` classes, providing user-friendly feedback on failures. - Updated the `CreateSnippet` class to handle database errors gracefully, ensuring robust error messaging for snippet creation. - Improved error handling in the `EncodeDecode` and `Ping` classes to capture specific exceptions and log errors appropriately. - Streamlined the `Poll` class to include error handling for poll ban checks, ensuring smoother user experience during poll creation.
- Updated the default value of the 'dirty' option in the DocsCLI class from True to False, ensuring that all files are rebuilt by default during the documentation serving process.
…n component styling - Implemented smooth scrolling behavior across the site for improved navigation. - Added custom scrollbar styles for a more modern look, including thin scrollbars and color adjustments. - Updated header, footer, and button styles to align with the Tokyo Night theme. - Enhanced typography and list formatting for better readability. - Introduced new styles for hero sections, feature grids, and navigation components to improve overall layout and user experience.
- Introduced a new MkDocs plugin for Tux bot documentation that utilizes AST parsing to extract command information. - Implemented configuration options for module paths and command enabling. - Added functionality to process command blocks in markdown and generate documentation based on command definitions. - Enhanced command documentation with details such as usage, parameters, and permission levels.
- Added a new accent color (sky) to the theme for improved visual appeal. - Introduced the Tux plugin configuration to enable command documentation generation. - Restructured the navigation to include detailed user, development, administration, and community documentation sections, enhancing accessibility and organization. - Updated markdown extensions to include emoji support and tabbed content for better user experience.
Update the README to reflect changes in CLI tools and plugin system. Replace `click` with `typer` for CLI and update the extensions system to a plugin system. Provide detailed instructions for using the new CLI commands with `uv`. Add new documentation files for contributing, FAQ, support, database patterns, error handling, Sentry integration, and an admin guide. These documents provide comprehensive guidelines for development, error tracking, and administration of the Tux bot. The changes aim to improve developer experience by providing clear guidelines and tools for contributing and maintaining the project. The new documentation supports better onboarding, troubleshooting, and system management, ensuring consistency and reliability in development and production environments. docs: add comprehensive developer and user guides Introduce detailed developer and user guides to enhance contributor and user experience. The developer guide provides step-by-step instructions for setting up a development environment, contributing to the project, and understanding the architecture. The user guide offers instructions for server administrators and members on how to install, configure, and use Tux. These guides aim to streamline the onboarding process for new contributors and users, ensuring they have all necessary information to effectively engage with the project. The documentation also includes API references and CLI tools to support advanced usage and development workflows.
Introduce a new entry point for the MkDocs plugin, allowing the integration of the TuxPlugin. This change facilitates the use of custom documentation features provided by the TuxPlugin, enhancing the documentation generation process.
…witch branches often
…eveloper documentation Refactor the markdown formatting across multiple documentation files to improve readability and consistency. Changes include removing unnecessary text formatting tags and ensuring proper code block syntax. This update enhances the overall presentation of the documentation, making it easier for contributors and users to follow the guidelines and instructions provided.
Changed the import path for the ErrorHandler from `tux.services.handlers.error.handler` to `tux.services.handlers.error.cog` in both end-to-end and unit tests. This update ensures consistency in the codebase and aligns with the recent refactoring of the error handling module.
…ation Add multiple setup services for bot initialization, including BaseSetupService, BotSetupService, CogSetupService, DatabaseSetupService, PermissionSetupService, and BotSetupOrchestrator. These services standardize the setup process, handle error logging, and manage dependencies for cogs, database connections, and permissions, enhancing the overall bot setup experience.
…unloading Implement a new method in the BaseCog class to unload the cog if a specified configuration is missing. This method logs a warning and triggers an asynchronous unload task, enhancing error handling and configuration management for cogs.
…hods Refactor the Tux bot class to enhance the setup process by introducing a dedicated method for creating the setup task in the proper event loop context. Removed obsolete methods related to setup steps, including database connection handling and permission system initialization, which are now managed by the BotSetupOrchestrator. This change simplifies the bot's initialization logic and improves maintainability.
Refactor the cog eligibility verification process in the CogLoader class to include checks for the presence of a setup function in the module. This update improves the robustness of cog loading by ensuring only valid cogs are loaded. Additionally, enhance logging to provide clearer feedback on the loading process, including the number of cogs loaded from each folder and their respective load times.
…ed parameters Refactor the logging configuration in the logging module to streamline the setup process. The `environment` parameter is now deprecated and kept for backward compatibility, while the logging level is simplified to use a single source. Additionally, the console format function has been updated to remove environment-specific logic, enhancing clarity and maintainability.
Refactor the `get_db_service_from` function to enhance the logic for retrieving the DatabaseService. The updated implementation first checks the bot's container for the service and falls back to directly accessing the db_service attribute if not found. This change improves the robustness of the service retrieval process and maintains clearer error handling.
Add the unload_if_missing_config method to various cogs, including Git, Levels, and StatusRoles, to ensure graceful unloading when required configurations are missing. This enhancement improves error handling and configuration management across the bot's modules.
…iscord commands Add a new ErrorHandler cog that centralizes error handling for both prefix and slash commands in the Tux Discord bot. This implementation includes enhanced logging, Sentry integration for error reporting, and user-friendly error responses. Update import paths to reflect the new structure.
…substitution and improved activity rotation Refactor the ActivityHandler to include a new method for placeholder substitution in activity names, allowing dynamic updates based on bot statistics. Update the activity list building logic to handle empty configurations gracefully and streamline the activity rotation process. This enhancement improves the bot's presence management and user engagement.
…ebase This commit deletes the substitutions.py file, which contained methods for handling dynamic text substitutions based on bot statistics. The removal is part of a cleanup effort to streamline the codebase and eliminate unnecessary components.
This commit introduces a new hot reload cog for file watching and automatic reloading of extensions. The cog setup function is added to facilitate integration with the bot. Additionally, the reload logic in the HotReload service and CogWatcher class is improved to prevent attempts to reload when the event loop is closed, enhancing stability during shutdown.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. If this change fixes any issues please put "Fixes #XX" in the description. Please also ensure to add the appropriate labels to the PR.
Guidelines
My code follows the style guidelines of this project (formatted with Ruff)
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 if needed
My changes generate no new warnings
I have tested this change
Any dependent changes have been merged and published in downstream modules
I have added all appropriate labels to this PR
I have followed all of these guidelines.
How Has This Been Tested? (if applicable)
Please describe how you tested your code. e.g describe what commands you ran, what arguments, and any config stuff (if applicable)
Screenshots (if applicable)
Please add screenshots to help explain your changes.
Additional Information
Please add any other information that is important to this PR.
Summary by Sourcery
Migrate project from Poetry to Uv (Hatch) as the dependency and build tool, restructure code under src/, introduce a dependency injection container with BaseCog for service resolution, centralize Sentry integration via a new SentryManager, and overhaul tracing/error handling instrumentation. Update CLI commands, Docker configurations, CI workflows, documentation, and tests to use Uv, add unit/integration/e2e test markers with isolation, and bump version to v0.1.0.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: