Skip to content
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

[change] Move python dependencies to requirements.txt file #511

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

shwetd19
Copy link

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

Closes #507

Description of Changes

This PR here implements automated dependency management using Dependabot by extracting Python dependencies from pip.yml into a dedicated requirements.txt file.

Key Changes:

  1. New Requirements File

    • Created files/requirements.txt containing all Python dependencies
    • Organized dependencies by category with clear comments
    • Maintained version constraints where necessary
  2. Updated Package Installation

    • Modified tasks/pip.yml to use the new requirements file
    • Implemented version constraints using pip's constraint feature
    • Preserved compatibility with existing Ansible variables
    • Maintained support for conditional package installation
  3. Dependabot Configuration

    • Added Python package ecosystem configuration
    • Set weekly update schedule
    • Configured PR labeling and commit message format

Testing Details:

  1. Installation Testing

    Tested with various configurations:

    molecule test -s local
  2. Integration Testing

    • Validated Dependabot configuration
    • Tested requirements file parsing
    • Verified dependency resolution

Benefits:

  • Automated dependency updates via Dependabot
  • Improved dependency management
  • Maintained backward compatibility
  • Better organized package requirements

Screenshots

image

- Modified pip.yml to install dependencies from requirements.txt
- Used role_path variable to reference requirements.txt in files directory
- Implemented pip constraints to maintain version control from Ansible variables
- Kept an extra_python_packages handling for custom additions
- and preserved existing retry logic and environment settings
- Here grouped dependencies by their functionality (core, database, optional)
- Added few comments to indicate optional packages and their purpose
- While maintaining existing version constraints for critical packages
- I've ensured compatibility with current OpenWISP installation process
- Added pip package ecosystem to dependabot configuration
- While setting up weekly update schedule for dependency checks
- Added all 'dependencies' and 'python' labels for better PR organization
interval: "monthly" # Check for updates weekly
commit-message:
prefix: "[ci] "
prefix: "deps"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
prefix: "deps"
prefix: "[deps]"

commit-message:
prefix: "[deps] "
- package-ecosystem: "github-actions" # Check for GitHub Actions updates
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing github-actions? Please don't.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made the fix for this in my recent commit

- Maintained consistent prefix formatting using square brackets: [deps] for pip and [ci] for GitHub Actions
- Restored the GitHub Actions ecosystem configuration
- Setup both update intervals to "weekly" as originally intended
@shwetd19 shwetd19 requested a review from nemesifier February 4, 2025 05:41
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shwetd19 the build is failing. Please execute the tests locally before pushing.

@shwetd19
Copy link
Author

shwetd19 commented Feb 7, 2025

Hey @nemesifier ,

The main issue was that the requirements.txt file cannot be found during the installation process. The error message was consistent across different Ubuntu and Debian environments I've made the possible fix and did tested it properly this time

can you please review my PR

@shwetd19 shwetd19 requested a review from nemesifier February 7, 2025 04:15
@shwetd19
Copy link
Author

Hey @pandafy @nemesifier can you please reivew this PR ?
pls let me know if any changes are required, Thanks!

Copy link
Member

@pandafy pandafy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's only extract the Python dependencies which are pinned to a version. Take a note to respect the when condition of the existing tasks that installs Python modules. The role does not install all Python packages everytime.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need any changes for this file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed dependabot as it was previously in my latest commit

Comment on lines 12 to 13
psycopg2
MySQL-python
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These packages are installed only when specific database backends are used. Since, we are installing the latest version of these packages, adding them here is not fruitful.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified requirements.txt file for removing Database adapters

tasks/pip.yml Outdated
Comment on lines 77 to 60
state: latest
state: present
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change this? latest allows to upgrade the Python modules when using compatibility versioning. Same for other instances.

Copy link
Author

@shwetd19 shwetd19 Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified pip.yml for database adapters psycopg2 & MySQL-python and to fix the state as latest

@shwetd19 shwetd19 requested a review from pandafy February 11, 2025 14:01
@shwetd19
Copy link
Author

Hello @pandafy I've implemented the suggested changes in my recent commits, can you please check that once, Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

[enhancement] Use dependabot to upgrade dependencies in pip.yml
3 participants