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

52 python dependencies to requirements #60

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

shwetd19
Copy link

Checklist


Reference to Existing Issue

Closes #52.


Description of Changes

This pull request moves Python dependencies from pip.yml to a requirements.txt file for easier management. Here’s what was done:

  1. Created a requirements.txt file:

    • Added all dependencies, such as Flask, Werkzeug, and uwsgi, into this file to keep things organized.
  2. Updated pip.yml:

    • Adjusted the Ansible tasks to install dependencies from requirements.txt instead of listing them directly in the file.
    • Added steps to:
      • Update pip and related tools like setuptools and wheel.
      • Install dependencies in a virtual environment using the requirements.txt file.
  3. Dependabot Integration:

    • Made sure tools like Dependabot can track and update dependencies in requirements.txt automatically.
  4. Tested the Changes:

    • Ran the updated playbook to confirm everything works as expected.

These changes make it easier to manage and update dependencies while keeping the setup clean and consistent.

Added all dependencies, such as Flask, Werkzeug, and uwsgi, into this file to keep things organized.
Adjusted the Ansible tasks to install dependencies from requirements.txt instead of listing them directly in the file & updated pip and related tools like setuptools and wheel.
tasks/pip.yml Outdated
Comment on lines 23 to 19
- name: Install Flask, Werkzeug and uWSGI
- name: Read local requirements.txt
local_action: command cat requirements.txt
register: requirements
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this is correct? Did you try running the tests locally?
https://github.com/openwisp/ansible-wireguard-openwisp?tab=readme-ov-file#how-to-run-tests

Using `{{ role_path }}` instead of absolute paths, this variable points to the root directory of our Ansible role, where requirements.txt is located.
@shwetd19
Copy link
Author

@pandafy , I've fixed the path for requirements.txt file, now it's working.. can you please check nd lmk, Thanks!

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.

See my comment below, please also take a look at the message returned by the QA checks failure regarding the lack of capital letter after the square bracket, eg:

[prefix] done something should be [prefix] Done something.

- wheel
- attrs
- importlib-metadata
- packaging
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't move these because we're always upgrade to the latest version available.

The aim is to move only the python packages that we are pinning to as pecific version range, so that dependabot can help us update those more with less intervention from our end.

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

Successfully merging this pull request may close these issues.

[change] Move Python dependencies to requirements.txt
3 participants