feat: Phase 1 - Project Setup & Core Infrastructure #1
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
Phase 1 implementation: - Add pyproject.toml with all dependencies and tool configs - Add requirements.txt for production dependencies - Add requirements-dev.txt for development dependencies - Add .env.example with environment variable template - Add GitHub Actions CI workflow (lint, type-check, test) - Add tests/conftest.py with shared fixtures - Create package structure for all components - Configure ruff, mypy, pytest, and coverage 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer's GuideSets up the initial Python project structure and core infrastructure, including packaging metadata, dependencies, CI pipeline, and shared pytest fixtures for network-related tools in a sysadmin portfolio. Class diagram for the initial sysadmin portfolio package structureclassDiagram
class SysadminToolkitPackage {
+str __version__ = "1.0.0"
+str __author__ = "SysAdmin Portfolio"
+get_system_info()
}
class NetworkHealthCheckerPackage {
+str __version__ = "1.0.0"
+str __author__ = "SysAdmin Portfolio"
+ping_host(host)
}
class BackupAutomationPackage {
+str __version__ = "1.0.0"
+str __author__ = "SysAdmin Portfolio"
+FileBackup(source, dest)
+run_backup()
}
class ProjectConfiguration {
+str name = "sysadmin-portfolio"
+str version = "1.0.0"
+str requires_python = ">=3.9"
+dependencies
+dev_dependencies
+configure_ruff()
+configure_mypy()
+configure_pytest()
+configure_coverage()
}
ProjectConfiguration "1" o-- "1" SysadminToolkitPackage : configures
ProjectConfiguration "1" o-- "1" NetworkHealthCheckerPackage : configures
ProjectConfiguration "1" o-- "1" BackupAutomationPackage : configures
Flow diagram for CI test job across Python versionsflowchart TD
A["Start test job\n(push or PR to main)"] --> B["Create matrix of Python versions\n3.9, 3.10, 3.11, 3.12"]
B --> C["Checkout repository"]
C --> D["Set up Python for matrix version"]
D --> E["Install dev dependencies\nrequirements-dev.txt"]
E --> F["Run pytest with coverage\npytest tests/ -v --cov --cov-report=xml"]
F --> G{Is version 3.11?}
G -- Yes --> H["Upload coverage.xml to Codecov"]
G -- No --> I["Skip Codecov upload"]
H --> J["Matrix job complete"]
I --> J
J --> K["All matrix runs finished"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. WalkthroughA new SysAdmin Portfolio project is established with configuration templates, CI/CD automation, Python package structure with metadata, dependency management, and comprehensive testing infrastructure including pytest fixtures and mocking scaffolding. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- The test setup currently modifies sys.path in tests/conftest.py; given that pytest is already configured with pythonpath=["."], consider removing the manual PROJECT_ROOT insertion and relying on standard package discovery to avoid surprising import behavior.
- There is a mismatch between the configured package names and the filesystem layout (e.g., tool.setuptools.packages.find includes "network_health_checker*" while the directory is "3-network-health-checker/"); aligning the directory names with the package names (or adding appropriate package dirs) will prevent import and packaging issues, especially for the nettools entry point.
- Pytest markers are defined both in pyproject.toml (slow, integration) and again in pytest_configure (slow, integration, network); to keep marker configuration maintainable, consider consolidating marker definitions in a single place (preferably pyproject.toml) and adding the missing
networkmarker there instead of duplicating them in code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The test setup currently modifies sys.path in tests/conftest.py; given that pytest is already configured with pythonpath=["."], consider removing the manual PROJECT_ROOT insertion and relying on standard package discovery to avoid surprising import behavior.
- There is a mismatch between the configured package names and the filesystem layout (e.g., tool.setuptools.packages.find includes "network_health_checker*" while the directory is "3-network-health-checker/"); aligning the directory names with the package names (or adding appropriate package dirs) will prevent import and packaging issues, especially for the nettools entry point.
- Pytest markers are defined both in pyproject.toml (slow, integration) and again in pytest_configure (slow, integration, network); to keep marker configuration maintainable, consider consolidating marker definitions in a single place (preferably pyproject.toml) and adding the missing `network` marker there instead of duplicating them in code.
## Individual Comments
### Comment 1
<location> `pyproject.toml:66-71` </location>
<code_context>
+Repository = "https://github.com/w7-mgfcode/sysadmin-portfolio.git"
+Issues = "https://github.com/w7-mgfcode/sysadmin-portfolio/issues"
+
+[tool.setuptools.packages.find]
+where = ["."]
+include = [
+ "network_health_checker*",
+ "sysadmin_toolkit*",
+ "backup_automation*",
+]
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Package discovery patterns likely won't match the actual directory structure with numeric prefixes.
The `include` patterns expect top-level packages named `network_health_checker*`, `sysadmin_toolkit*`, and `backup_automation*`, but the repo directories use numeric prefixes (e.g. `1-sysadmin-toolkit`, `3-network-health-checker`). Unless there are matching package dirs without prefixes, these modules won’t be included in the built distribution. Consider either adding correctly named package directories (e.g. `network_health_checker/` inside `3-network-health-checker/`) or updating `where`/`include` to match the actual importable package paths.
</issue_to_address>
### Comment 2
<location> `pyproject.toml:57-58` </location>
<code_context>
+ "black>=23.0.0",
+]
+
+[project.scripts]
+nettools = "network_health_checker.cli:app"
+
+[project.urls]
</code_context>
<issue_to_address>
**issue (bug_risk):** The console script entry point may not resolve if the `network_health_checker` package/module isn't actually importable at the top level.
This entry point targets `network_health_checker.cli:app`, but the repo layout (e.g. `3-network-health-checker/`) doesn’t clearly provide a top-level `network_health_checker` package. Without a real `network_health_checker/cli.py` importable on `PYTHONPATH`, running `nettools` will raise `ImportError`. Please either add a matching `network_health_checker` package with `cli.py` (and `app`) or update the entry point to the actual module path.
</issue_to_address>
### Comment 3
<location> `pyproject.toml:127-132` </location>
<code_context>
+branch = true
+omit = ["*/tests/*", "*/__init__.py"]
+
+[tool.coverage.report]
+exclude_lines = [
+ "pragma: no cover",
+ "def __repr__",
+ "raise NotImplementedError",
+ "if __name__ == .__main__.:",
+ "if TYPE_CHECKING:",
+]
</code_context>
<issue_to_address>
**issue (testing):** The coverage exclude pattern for `__main__` appears malformed and likely won't match.
This entry looks intended to match the usual `if __name__ == "__main__":` guard, but the dots and missing quotes mean it won’t match real code, so those blocks won’t be excluded as expected. Please update this to a correct string/regex pattern (e.g. `"if __name__ == \"__main__\":"`) so coverage behaves as intended.
</issue_to_address>
### Comment 4
<location> `tests/conftest.py:38` </location>
<code_context>
@pytest.fixture(autouse=True)
def setup_test_environment() -> Generator[None, None, None]:
"""
Teszt környezet beállítása minden teszthez.
Beállítja a szükséges környezeti változókat és
a teszt végén visszaállítja az eredeti állapotot.
"""
# Eredeti környezet mentése
original_env = os.environ.copy()
# Teszt környezeti változók beállítása
os.environ.update(
{
"SNMP_COMMUNITY": "public",
"SNMP_VERSION": "2c",
"DEFAULT_TIMEOUT": "5.0",
"LOG_LEVEL": "DEBUG",
}
)
yield
# Eredeti környezet visszaállítása
os.environ.clear()
os.environ.update(original_env)
</code_context>
<issue_to_address>
**issue (code-quality):** Merge dictionary updates via the union operator [×2] ([`dict-assign-update-to-union`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/dict-assign-update-to-union/))
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+66
to
+71
| [tool.setuptools.packages.find] | ||
| where = ["."] | ||
| include = [ | ||
| "network_health_checker*", | ||
| "sysadmin_toolkit*", | ||
| "backup_automation*", |
There was a problem hiding this comment.
issue (bug_risk): Package discovery patterns likely won't match the actual directory structure with numeric prefixes.
The include patterns expect top-level packages named network_health_checker*, sysadmin_toolkit*, and backup_automation*, but the repo directories use numeric prefixes (e.g. 1-sysadmin-toolkit, 3-network-health-checker). Unless there are matching package dirs without prefixes, these modules won’t be included in the built distribution. Consider either adding correctly named package directories (e.g. network_health_checker/ inside 3-network-health-checker/) or updating where/include to match the actual importable package paths.
Comment on lines
+57
to
+58
| [project.scripts] | ||
| nettools = "network_health_checker.cli:app" |
There was a problem hiding this comment.
issue (bug_risk): The console script entry point may not resolve if the network_health_checker package/module isn't actually importable at the top level.
This entry point targets network_health_checker.cli:app, but the repo layout (e.g. 3-network-health-checker/) doesn’t clearly provide a top-level network_health_checker package. Without a real network_health_checker/cli.py importable on PYTHONPATH, running nettools will raise ImportError. Please either add a matching network_health_checker package with cli.py (and app) or update the entry point to the actual module path.
Comment on lines
+127
to
+132
| [tool.coverage.report] | ||
| exclude_lines = [ | ||
| "pragma: no cover", | ||
| "def __repr__", | ||
| "raise NotImplementedError", | ||
| "if __name__ == .__main__.:", |
There was a problem hiding this comment.
issue (testing): The coverage exclude pattern for __main__ appears malformed and likely won't match.
This entry looks intended to match the usual if __name__ == "__main__": guard, but the dots and missing quotes mean it won’t match real code, so those blocks won’t be excluded as expected. Please update this to a correct string/regex pattern (e.g. "if __name__ == \"__main__\":") so coverage behaves as intended.
| original_env = os.environ.copy() | ||
|
|
||
| # Teszt környezeti változók beállítása | ||
| os.environ.update( |
There was a problem hiding this comment.
issue (code-quality): Merge dictionary updates via the union operator [×2] (dict-assign-update-to-union)
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.
Summary
Phase 1 implementation - Project setup and core infrastructure.
What was implemented
pyproject.toml: Complete project configuration with:
Requirements files:
requirements.txt- Production dependenciesrequirements-dev.txt- Development dependenciesEnvironment configuration:
.env.example- Template with all environment variablesCI/CD Pipeline:
Test infrastructure:
tests/conftest.py- Shared fixtures for mocking network operationsPackage structure:
1-sysadmin-toolkit/3-network-health-checker/network_tools/5-backup-automation/Main files
pyproject.tomlrequirements.txtrequirements-dev.txt.env.example.github/workflows/ci.ymltests/conftest.pyPhase 1 Checklist
🤖 Generated with Claude Code
Summary by Sourcery
Set up the initial Python project structure, tooling, and automation for the sysadmin portfolio monorepo.
New Features:
Enhancements:
Build:
CI:
Tests:
Chores:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.