Migrate to Poetry and Makefile-based workflow #252
Conversation
Adjusted comment formatting in dependabot workflow for readability and added missing newlines at end of YAML files to comply with linting and formatting standards. Migrate to Poetry and Makefile-based workflow Replaces tox and requirements files with Poetry for dependency management and a Makefile for task automation. Updates CI/CD to use GitHub Actions with Poetry and Make, adds Python 3.13 support, and drops support for Python 3.9/3.10. Updates Dockerfile, .gitignore, and documentation to reflect the new workflow. Removes legacy files including setup.py, tox.ini, Travis CI, and requirements files.
|
⏳ Code review in progress. Analyzing for code quality issues and best practices. Detailed findings will be posted upon completion. Using Amazon Q Developer for GitHubAmazon Q Developer1 is an AI-powered assistant that integrates directly into your GitHub workflow, enhancing your development process with intelligent features for code development, review, and transformation. Slash Commands
FeaturesAgentic Chat Code Review CustomizationYou can create project-specific rules for Amazon Q Developer to follow:
Example rule: FeedbackTo provide feedback on Amazon Q Developer, create an issue in the Amazon Q Developer public repository. For more detailed information, visit the Amazon Q for GitHub documentation. Footnotes
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #252 +/- ##
==========================================
+ Coverage 78.48% 83.43% +4.95%
==========================================
Files 4 4
Lines 158 163 +5
==========================================
+ Hits 124 136 +12
+ Misses 34 27 -7 🚀 New features to boost your workflow:
|
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR represents a significant modernization of the project's build and dependency management system. The migration from tox to Poetry with Makefile-based workflows is well-executed and brings several benefits:
✅ Strengths
- Modern Dependency Management: Poetry provides better dependency resolution and lock file management compared to the previous requirements.txt approach
- Improved CI/CD: The new GitHub Actions workflow is cleaner and more maintainable than the previous tox-based setup
- Python Version Updates: Adding Python 3.13 support while dropping EOL versions (3.9, 3.10) is appropriate
- Developer Experience: The Makefile provides a consistent interface for common development tasks
- Security Improvements: The datetime modernization fixes deprecation warnings and follows current best practices
- Docker Optimization: Better layer caching with separate dependency installation steps
🔧 Areas for Improvement
- Configuration Duplication: The pyproject.toml contains both PEP 517/518 and Poetry configurations, which could lead to maintenance overhead
- Dependabot Conflicts: Both pip and poetry ecosystems are configured, which might create conflicting updates
- Documentation Inconsistencies: Some references to old Python versions remain in the documentation
- Hardcoded Paths: The Dockerfile contains hardcoded Python version paths that could break with updates
📋 Recommendations
- Consider standardizing on either PEP 517/518 or Poetry configuration (not both)
- Review Dependabot configuration to avoid ecosystem conflicts
- Update all documentation to reflect the new supported Python versions
- Use dynamic path resolution in Docker for better maintainability
Overall, this is a solid modernization effort that significantly improves the project's maintainability and developer experience. The changes are well-structured and follow current Python packaging best practices.
pyproject.toml
Outdated
| # Poetry-specific configuration (for compatibility) | ||
| [tool.poetry] | ||
| name = "ssllabsscan" | ||
| version = "4.0.0" | ||
| description = "SSL Labs Analysis Reporting" | ||
| authors = ["Kay Hau <virtualda@gmail.com>"] | ||
| license = "MIT" | ||
| readme = "README.md" | ||
| packages = [{include = "ssllabsscan"}] | ||
| exclude = ["ssllabsscan/tests"] | ||
|
|
||
| [tool.poetry.dependencies] | ||
| python = ">=3.11,<4.0" | ||
| requests = "2.32.3" | ||
|
|
||
| [tool.poetry.group.test.dependencies] | ||
| pytest = "8.4.0" | ||
| pytest-cov = "7.0.0" | ||
| pytest-gitignore = "1.3.0" | ||
| pytest-mock = "3.15.0" | ||
| coverage = "7.10.6" | ||
| mock = "5.2.0" | ||
| flake8 = "7.3.0" | ||
|
|
||
| [tool.poetry.group.dev.dependencies] | ||
| setuptools = "80.9.0" | ||
| wheel = "0.45.0" | ||
| yamllint = "1.35.0" | ||
|
|
||
| [tool.poetry.scripts] | ||
| ssllabs-scan = "ssllabsscan.main:main" |
There was a problem hiding this comment.
The pyproject.toml file contains duplicate configuration sections for both PEP 517/518 standards and Poetry. This creates maintenance overhead and potential inconsistencies. Consider choosing one approach for cleaner configuration management.
# Note: Poetry configuration can be removed if using pure PEP 517/518 approach
# Alternatively, if keeping Poetry, consider removing the [project] section above
# and using Poetry's native configuration exclusively| rm -rf *.egg-info/ | ||
| rm -rf *.egg | ||
| rm -rf .pytest_cache/ | ||
| find . -type d -name __pycache__ -exec rm -rf {} + |
There was a problem hiding this comment.
The find command with -exec rm -rf {} + can fail if it encounters directories that are being processed. Consider using a safer approach that handles potential race conditions.
| find . -type d -name __pycache__ -exec rm -rf {} + | |
| find . -type d -name __pycache__ -print0 | xargs -0 rm -rf |
| with: | ||
| python-version: "3.13" | ||
| - name: Install Poetry | ||
| run: pip install poetry |
There was a problem hiding this comment.
Consider pinning Poetry to a specific version for reproducible builds and security. The current installation method uses the latest version which could introduce unexpected changes.
| run: pip install poetry | |
| - name: Install Poetry | |
| run: pip install poetry==1.8.5 |
| COPY . . | ||
|
|
||
| # Copy styles.css to the correct location | ||
| RUN cp sample/styles.css /usr/local/lib/python3.13/site-packages/ssllabsscan/styles.css |
There was a problem hiding this comment.
The hardcoded Python version path in the copy command creates a maintenance burden and could break if the Python version changes. Consider using a more dynamic approach or environment variable.
| RUN cp sample/styles.css /usr/local/lib/python3.13/site-packages/ssllabsscan/styles.css | |
| # Copy styles.css to the correct location | |
| RUN PYTHON_SITE_PACKAGES=$(python -c "import site; print(site.getsitepackages()[0])") && \ | |
| cp sample/styles.css "$PYTHON_SITE_PACKAGES/ssllabsscan/styles.css" |
| - package-ecosystem: poetry | ||
| directory: "/" | ||
| groups: | ||
| all-dependencies: | ||
| patterns: | ||
| - "*" | ||
| schedule: | ||
| interval: weekly | ||
| time: "09:30" | ||
| timezone: Australia/Melbourne | ||
| open-pull-requests-limit: 1 |
There was a problem hiding this comment.
Good addition of Poetry ecosystem support to Dependabot! However, consider whether both pip and poetry ecosystems are needed simultaneously, as this might create conflicting dependency updates. Since the project has migrated to Poetry, you may want to remove the pip ecosystem configuration.
|
|
||
| # Testing | ||
| make test # Run tests | ||
| make test-all # Run tests on all Python versions (3.9-3.12) |
There was a problem hiding this comment.
The documentation mentions Python versions 3.9-3.12 for testing, but the project now only supports Python 3.11+ according to the pyproject.toml. This should be updated to reflect the current supported versions.
| make test-all # Run tests on all Python versions (3.9-3.12) | |
| make test-all # Run tests on all Python versions (3.11-3.13) |
Introduces new test files for export_response and report_template modules, and significantly expands test coverage in test_main.py and test_ssllabs_client.py. The new and updated tests cover file creation, error handling, template structure, client initialization, and edge cases, improving overall reliability and maintainability.
Replaces tox and requirements files with Poetry for dependency management and a Makefile for task automation. Updates CI/CD to use GitHub Actions with Poetry and Make, adds Python 3.13 support, and drops support for Python 3.9/3.10. Updates Dockerfile, .gitignore, and documentation to reflect the new workflow. Removes legacy files including setup.py, tox.ini, Travis CI, and requirements files.