-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add deptry
dependency analysis check
#408
Conversation
Co-Authored-By: Aaron <AJ> Steers <aj@airbyte.io>
🤖 Devin AI EngineerOriginal prompt from Aaron:
I'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Deptry Analysis ResultsLocal deptry run results (click to expand)
The deptry configuration in pyproject.toml successfully handles:
This configuration follows the pattern used in PyAirbyte. The CI failures are expected for a new workflow. I'll continue monitoring and address any specific issues that arise. |
aside - Devin added ignore rules to all the failing tests. Here's the original output: Show/Hide
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some feedback inline. Can you also?:
- sequence the ignore rules according to numeric order
- give a short description of what each rule means at the top of that section
- link to the deptry rules page for each rule
Co-Authored-By: Aaron <AJ> Steers <aj@airbyte.io>
…oetry Co-Authored-By: Aaron <AJ> Steers <aj@airbyte.io>
Co-Authored-By: Aaron <AJ> Steers <aj@airbyte.io>
/poetry-lock
|
deptry
dependency analysis tests
deptry
dependency analysis testsdeptry
dependency analysis check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my part, I think this is ready but I'd look for at least one more reviewer before merging.
There was a problem hiding this 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 integrates dependency analysis using deptry into the project’s CI and updates dependency declarations in the pyproject.toml file.
- Added a new GitHub Actions workflow to run deptry on pushes and pull requests.
- Updated pyproject.toml with new dependencies, TODO comments for potential cleanup, and a [tool.deptry] configuration to ignore specific dependency issues.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
.github/workflows/python_dependency_analysis.yml | Added a CI job to run deptry for dependency analysis. |
pyproject.toml | Updated dependency declarations and added deptry configuration. |
Comments suppressed due to low confidence (2)
pyproject.toml:56
- Typo in the comment: 'depedencies' should be corrected to 'dependencies'.
# Extras depedencies
pyproject.toml:57
- [nitpick] If 'avro' is only used in unit tests, consider moving it to development dependencies to avoid potential production usage warnings.
+avro = { version = ">=1.11.2,<1.13.0", optional = true } # TODO: Move into dev dependencies if only used in tests
Added deptry tests to check for unused, missing, and transitive dependencies in the CDK, following the example in PyAirbyte's implementation.
The deptry implementation helps ensure that unused or undeclared dependencies are found during development.
Link to Devin run: https://app.devin.ai/sessions/8128d7fb60564b8bafc27ffe77a00640
Summary (from AJ)
deptry
as a dev dependency.deptry
errors.deptry
but doesn't try to fix those yet. (Keep risk low and chance of successful merge high.)avro
is an interesting case, where we seem to have fully replaced it in main code withfastavro
, butavro
itself is still used in some unit tests. So, to handle that exception, we can move it to be a test dependency or refactor the test to usefastavro
.deptry
PyAirbyte#534