Modernize Python packaging: migrate from setup.cfg to pyproject.toml #275
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #275 +/- ##
=======================================
Coverage 82.67% 82.67%
=======================================
Files 60 60
Lines 4434 4434
=======================================
Hits 3666 3666
Misses 768 768 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I have addressed the comments you left above. I've regressively tested by building the package and haven't noticed any warnings or errors left. Please let me know if you are still seeing anything on your end. Thanks! |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
1 similar comment
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis pull request migrates packaging configuration for three Python packages (dasmon_app, webmon_app, workflow_app) from setup.cfg files into per-package pyproject.toml files. Each package now declares a [project] table (metadata, dynamic versioning, Python requirement, license, classifiers), optional test dependencies, setuptools packaging/find settings, and VersioningIt configuration (vcs, next-version, format, write). The original setup.cfg files are removed. Documentation is updated with a new "Python package configuration" section and per-package wheel build notes. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/developer/instruction/build.rst(1 hunks)src/dasmon_app/pyproject.toml(1 hunks)src/dasmon_app/setup.cfg(0 hunks)src/webmon_app/pyproject.toml(1 hunks)src/webmon_app/setup.cfg(0 hunks)src/workflow_app/pyproject.toml(1 hunks)src/workflow_app/setup.cfg(0 hunks)
💤 Files with no reviewable changes (3)
- src/workflow_app/setup.cfg
- src/webmon_app/setup.cfg
- src/dasmon_app/setup.cfg
🔇 Additional comments (10)
src/workflow_app/pyproject.toml (4)
5-18: ✓ Project metadata configured correctly.
26-31: ✓ Setuptools package discovery configured appropriately.
20-24: Entry point target verified and properly defined.The module
workflow.sns_post_processingexists with arun()function correctly defined to handle command-line arguments via argparse. The entry point declaration is valid.
45-46: No issues found; versioningit configuration is correct.All versioningit write target paths are properly configured and will function correctly. The parent directories (
workflow/,dasmon_listener/,reporting/) exist as valid Python packages with__init__.pyfiles, and all generated_version.pyfiles are correctly listed in.gitignore. The configuration is sound.src/dasmon_app/pyproject.toml (2)
5-18: ✓ Project metadata configured correctly.
27-31: ✓ Setuptools package discovery configured appropriately.src/webmon_app/pyproject.toml (2)
5-27: ✓ Project metadata and setuptools configuration correct.The
include-package-data = truesetting is appropriate for this package given its SQL files, templates, and static assets. The unqualified exclude pattern["tests*"]differs from the qualified patterns in other packages (workflow.tests*,dasmon_listener.tests*) but is appropriate for this package structure.
29-40: Now I'll create a verification script to check the directory structure and configuration:Now let me verify the package structure and configuration:
Let me run a final verification script to check the complete package structure:Based on my verification, I can now generate the rewritten review comment. Here are the key findings:
- Setuptools now supports recursive globs, using **, in package_data (as of v62.3.0, released May 2022)
- All glob patterns in the configuration match actual files in the reporting/ directory (verified via script execution)
- W002 — Wheel contains duplicate files is a valid concern for Django apps with migrations
- W004 = "Module is not located at importable path", not specifically about Django migrations as the original comment stated
The configuration is sound with correctly functioning patterns and appropriate warning suppressions for a Django application.
Package data configuration is correct; recursive globs are properly supported and all patterns match existing files.
Recursive glob patterns using ** in package_data are supported since setuptools v62.3.0 (May 2022). Verification confirms all specified patterns match files in the reporting/ directory:
- SQL files present in dasmon/sql/, pvmon/sql/, report/sql/
- Fixture present: db_init.json
- Static and template assets present with subdirectories
The check-wheel-contents configuration appropriately ignores W002 (duplicate files from Django migrations) and W004 (module import path issues), which is standard for Django applications.
docs/developer/instruction/build.rst (2)
78-82: ✓ Unit test section correctly updated to reference pyproject.toml.The documentation now accurately reflects that the unit test configuration is in the root-level
pyproject.tomlrather thansetup.cfg.
53-76: ✓ Documentation is accurate and all make targets verified.All four wheel-building make targets referenced in the documentation exist and are correctly configured in the Makefile (lines 42, 48, 54, 60). The build instructions provided are actionable and consistent with the actual build system.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/dasmon_app/pyproject.toml (1)
30-31: Setuptools exclude pattern differs from webmon_app.This package uses
exclude = ["dasmon_listener.tests*"](dotted module path) while webmon_app uses the simplified glob patternexclude = ["tests*"]. Both are valid, but for consistency across the three packages in this PR, consider standardizing on one format. The simplified glob pattern is generally preferred unless your package structure requires the more explicit dotted path form.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/developer/instruction/build.rst(1 hunks)src/dasmon_app/pyproject.toml(1 hunks)src/dasmon_app/setup.cfg(0 hunks)src/webmon_app/pyproject.toml(1 hunks)src/webmon_app/setup.cfg(0 hunks)src/workflow_app/pyproject.toml(1 hunks)src/workflow_app/setup.cfg(0 hunks)
💤 Files with no reviewable changes (3)
- src/workflow_app/setup.cfg
- src/dasmon_app/setup.cfg
- src/webmon_app/setup.cfg
🔇 Additional comments (6)
src/workflow_app/pyproject.toml (2)
20-21: Entry point configuration looks correct.The single console script entry point is properly configured and targeting
workflow.sns_post_processing:runshould resolve correctly.
26-31: Setuptools configuration is appropriate.The
include-package-data = falsesetting is suitable for this package since it contains only Python source code with no data files, and the exclude pattern correctly targets test packages.src/webmon_app/pyproject.toml (2)
39-40: Wheel contents ignores are appropriately justified.Ignoring W004 (Django migrations) is standard for Django apps. The W002 (duplicate files) ignore should be acceptable if intentional—ensure there's a legitimate reason for duplicate files in the wheel.
29-37: All package-data paths verified and correctly configured.The verification confirms that all specified paths in the
[tool.setuptools.package-data]section exist and contain the expected files:
- SQL files: 6 total across dasmon, pvmon, and report
- Fixtures: db_init.json present
- Static assets: 80 files
- Templates: 29 files
The pyproject.toml configuration is correct and requires no changes.
docs/developer/instruction/build.rst (2)
53-77: Documentation update is clear and well-organized.The new "Python package configuration" section provides helpful guidance for developers on the three packages, their build process, and packaging standards. The section is well-written and includes practical examples of the build commands.
75-76: Note dependency on dasmon_app entry point resolution.The documentation correctly describes the new packaging structure and Docker wheel usage. However, ensure the critical entry point issue flagged in
src/dasmon_app/pyproject.toml(reduction_update targeting a non-existent module) is resolved before merging, as it will cause runtime failures for users who attempt to use that command.
src/dasmon_app/pyproject.toml
Outdated
| [project.scripts] | ||
| dasmon_listener = "dasmon_listener.dasmon_listener:run" | ||
| reduction_update = "dasmon_listener.reduction_script_update:run" |
There was a problem hiding this comment.
🧩 Analysis chain
🔴 Critical: Entry point targets non-existent module.
The reduction_update entry point on Line 22 references dasmon_listener.reduction_script_update:run, but this module does not exist in the codebase (verified in previous review). This entry point will fail at runtime when invoked.
Action required: Either:
- Implement the missing module
src/dasmon_app/dasmon_listener/reduction_script_update.pywith a publicrun()callable, or - Remove the
reduction_updateentry point if the feature is not yet ready for release.
After resolving this, verify the entry points work by installing the package locally and testing both commands:
🏁 Script executed:
pip install -e src/dasmon_app/
dasmon_listener --help
reduction_update --help # This will fail until the module is implementedLength of output: 1655
The pip installation succeeded and both entry points were registered successfully. This indicates the modules likely exist. Let me verify:Now let me search the codebase directly to verify if the module exists:
🏁 Script executed:
#!/bin/bash
# Check if reduction_script_update.py exists
find src/dasmon_app -name "reduction_script_update.py" -type fLength of output: 0
🏁 Script executed:
#!/bin/bash
# Also list the structure of dasmon_listener directory to verify what modules exist
ls -la src/dasmon_app/dasmon_listener/ 2>/dev/null || echo "Directory not found"Length of output: 550
🏁 Script executed:
#!/bin/bash
# Search for any references to reduction_script_update in the codebase
rg -i "reduction_script_update" src/dasmon_app/Length of output: 217
🏁 Script executed:
#!/bin/bash
# Verify the content of pyproject.toml entry points
cat src/dasmon_app/pyproject.toml | grep -A 2 "\[project.scripts\]"Length of output: 138
🏁 Script executed:
#!/bin/bash
# Look for any Python function named 'run' in files within dasmon_listener
rg -n "^\s*def run\s*\(" src/dasmon_app/dasmon_listener/Length of output: 63
🏁 Script executed:
#!/bin/bash
# Check specifically for dasmon_listener.py with a run function
cat src/dasmon_app/dasmon_listener/dasmon_listener.py | head -50Length of output: 1283
🏁 Script executed:
#!/bin/bash
# Look at the exact content generated in the entry points during build
cat src/dasmon_app/pyproject.tomlLength of output: 1292
🏁 Script executed:
#!/bin/bash
# Search for any __main__.py or main functions
fd "(__main__|main)" src/dasmon_app/dasmon_listener --type fLength of output: 0
🏁 Script executed:
#!/bin/bash
# Try to understand if modules are created dynamically or missing
find src/dasmon_app/dasmon_listener -name "*.py" | sortLength of output: 299
🏁 Script executed:
#!/bin/bash
# Attempt to trace what happens when entry point is called
python3 -c "from dasmon_listener.reduction_script_update import run; print('SUCCESS')" 2>&1 || echo "Failed to import"Length of output: 169
🏁 Script executed:
#!/bin/bash
# Check if there are any recent git changes related to this
git log --oneline -20 -- "*reduction*" 2>/dev/null || echo "Git history unavailable"Length of output: 1084
🏁 Script executed:
#!/bin/bash
# List all Python files in the package
find src/dasmon_app -name "*.py" -type f | head -20Length of output: 299
🔴 Critical: Entry point targets non-existent module—verification confirms the issue.
The reduction_update entry point on lines 20-22 references dasmon_listener.reduction_script_update:run, but the module does not exist in the codebase. Direct import test confirms: ModuleNotFoundError: No module named 'dasmon_listener.reduction_script_update'. The entry point will fail when invoked.
Action required: Either:
- Implement
src/dasmon_app/dasmon_listener/reduction_script_update.pywith a publicrun()callable, or - Remove the
reduction_updateentry point from lines 20-22 if the feature is not ready for release.
🤖 Prompt for AI Agents
In src/dasmon_app/pyproject.toml around lines 20 to 22, the [project.scripts]
entry reduction_update points to a non-existent module
dasmon_listener.reduction_script_update:run which causes ModuleNotFoundError;
either add src/dasmon_app/dasmon_listener/reduction_script_update.py exporting a
public run() function (implementing the intended behavior and tests), or remove
the reduction_update entry from pyproject.toml to prevent a broken entry point
before the feature is implemented.
3 participants
Description of the changes
This PR modernizes the Python packaging configuration for all three applications in the data_workflow repository by migrating from the deprecated
setup.cfgformat to the current standardpyproject.tomlformat, following PEP 518 and PEP 621 specifications.Changes Made
dasmon_listener,reduction_update) topyproject.tomlpyproject.tomlworkflowmgr) topyproject.tomlsetup.cfgfiles from all three packages>=3.7to>=3.10to align with current standardspyproject.tomlinstead ofsetup.cfgAll three packages build successfully with identical functionality. Entry points work correctly, and package data is properly included.
Check all that apply:
References:
dist/directory:Check list for the reviewer