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

shwetd19 · 2025-01-23T10:31:04Z

Checklist

I have read the [OpenWISP Contributing Guidelines](http://openwisp.io/docs/developer/contributing.html).
I have tested the changes in this pull request to make sure they work.
I have added or updated tests for any new or changed code.
I have updated the documentation where needed.

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:

Created a requirements.txt file:
- Added all dependencies, such as Flask, Werkzeug, and uwsgi, into this file to keep things organized.
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.
Dependabot Integration:
- Made sure tools like Dependabot can track and update dependencies in requirements.txt automatically.
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.

pandafy · 2025-01-23T10:46:49Z

tasks/pip.yml

- name: Install Flask, Werkzeug and uWSGI
+- name: Read local requirements.txt
+  local_action: command cat requirements.txt
+  register: requirements


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 · 2025-01-23T20:37:54Z

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

nemesifier

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.

nemesifier · 2025-01-24T15:10:22Z

tasks/pip.yml

-      - wheel
-      - attrs
-      - importlib-metadata
-      - packaging


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.

( `setuptools`, `wheel`, `attrs`, `importlib-metadata`, `packaging`) are removed from req file

- I've ensured `setuptools`, `wheel`, `attrs`, `importlib-metadata`, and `packaging` are installed directly via pip task - Replaced reading `requirements.txt` via `local_action` with explicit file copy to virtualenv - Changed package installation to use the copied `requirements.txt` file - Added condition to install packages only if `requirements.txt` changes

shwetd19 · 2025-01-24T20:10:46Z

Hey @pandafy @nemesifier I've modified the changes can you please review this once. Thanks!

Removed unnecessary dependencies (setuptools, wheel, attrs, importlib-metadata, packaging) from requirements.txt
Updated pip.yml:
- Ensured setuptools, wheel, attrs, importlib-metadata, and packaging are installed directly via pip task
- Replaced reading requirements.txt via local_action with explicit file copy to virtualenv
- Changed package installation to use the copied requirements.txt file
- Added condition to install packages only if requirements.txt changes
I've also tested it locally this time, it worked!

nemesifier

Great progress @shwetd19!

See my comments below.
Again I want to bring to your attention to 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.

tasks/pip.yml

requirements.txt

- Kept --- line - Maintained all other functionality

shwetd19 · 2025-01-26T05:09:05Z

@nemesifier I've made the suggested changes, please let me know if any changes are required, also pls forgive me if I'm making silly mistakes here, it's just I'm testing everything I can locally..

shwetd19 · 2025-01-27T12:21:15Z

Hey @pandafy @nemesifier can you please test this with QA checks.. nd let me know if any changes are required also can you pls tell me where I can contribute more ?

nemesifier

Small adjustments for consistency

tasks/pip.yml

shwetd19 · 2025-01-27T14:14:41Z

@nemesifier Thanks you very much for this! I'll try to learn as much as I can for never miss an oversight like this

nemesifier

LGTM!

pandafy

I took a closer look and found the following area for improvements.

pandafy · 2025-01-27T15:00:47Z

requirements.txt

This file should be inside the files/ directory.

Good point, thanks!

Moved requirements.txt from the root directory to files/requirements.txt

tasks/pip.yml

pandafy · 2025-01-27T15:04:17Z

tasks/pip.yml

@@ -36,3 +38,4 @@
  delay: 10
  register: result
  until: result is success
+  when: requirements_copy is changed


This should be run always.

Suggested change

when: requirements_copy is changed

shwetd19 · 2025-01-27T17:14:00Z

Hey @nemesifier @pandafy I've made the suggested changes

Moved requirements.txt from the root directory to files/requirements.txt
Updated the copy task to use the correct source path (requirements.txt will automatically look in the files/ directory)
Changed the destination path to {{ openwisp2_wireguard_path }}/requirements.txt instead of putting it in the virtualenv
Removed the when: requirements_copy is changed condition so pip install runs always
Updated the pip install task to use the requirements file from the new location

I've tested the code locally too at my end, pls let me know if any change is required, Thanks!

tasks/pip.yml

shwetd19 · 2025-01-28T10:07:48Z

Hey @nemesifier @pandafy both suggested fixes are done! pls can you check once

pandafy

LGTM!

Thanks for contributing @shwetd19 🚀

shwetd19 added 2 commits January 23, 2025 15:54

[change] added a requirements.txt file in preview openwisp#52

9365a1e

Added all dependencies, such as Flask, Werkzeug, and uwsgi, into this file to keep things organized.

[change] modified pip.yml to install packages from requirements file

c4c743a

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.

pandafy requested changes Jan 23, 2025

View reviewed changes

[change] modified pip.yml to fix the path of requirements file

99cadd0

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

nemesifier requested changes Jan 24, 2025

View reviewed changes

shwetd19 added 2 commits January 25, 2025 01:38

[change] Removed unnecessary dependencies

db20785

( `setuptools`, `wheel`, `attrs`, `importlib-metadata`, `packaging`) are removed from req file

nemesifier changed the title ~~52 python dependencies to requirements~~ [change] Move python dependencies to requirements.txt file Jan 24, 2025

nemesifier requested changes Jan 24, 2025

View reviewed changes

tasks/pip.yml Outdated Show resolved Hide resolved

tasks/pip.yml Outdated Show resolved Hide resolved

requirements.txt Outdated Show resolved Hide resolved

shwetd19 added 2 commits January 26, 2025 10:36

[change] Modified pip.yml for suggested changes

abf857c

- Kept --- line - Maintained all other functionality

[change] Updated uwsgi version

0bb285d

nemesifier requested changes Jan 27, 2025

View reviewed changes

tasks/pip.yml Outdated Show resolved Hide resolved

tasks/pip.yml Outdated Show resolved Hide resolved

tasks/pip.yml Outdated Show resolved Hide resolved

tasks/pip.yml Outdated Show resolved Hide resolved

nemesifier added 4 commits January 27, 2025 11:05

[chores] Update tasks/pip.yml

de62107

[chores] Update tasks/pip.yml

48858ef

[chores] Update tasks/pip.yml

0b304a6

[chores] Update tasks/pip.yml

769c15a

nemesifier assigned shwetd19 Jan 27, 2025

shwetd19 requested a review from nemesifier January 27, 2025 14:21

nemesifier approved these changes Jan 27, 2025

View reviewed changes

shwetd19 requested a review from pandafy January 27, 2025 14:59

pandafy requested changes Jan 27, 2025

View reviewed changes

[change] Modified pip.yml for suggested changes

32390c8

shwetd19 requested a review from pandafy January 27, 2025 17:14

nemesifier requested changes Jan 27, 2025

View reviewed changes

tasks/pip.yml Outdated Show resolved Hide resolved

tasks/pip.yml Outdated Show resolved Hide resolved

nemesifier mentioned this pull request Jan 27, 2025

[enhancement] Use dependabot to upgrade dependencies in pip.yml openwisp/ansible-openwisp2#507

Open

[change] Modified pip.yml for suggested changes

020b566

shwetd19 requested a review from nemesifier January 28, 2025 10:07

[chores] Improved pip.yml

93658ef

pandafy approved these changes Jan 28, 2025

View reviewed changes

nemesifier approved these changes Jan 28, 2025

View reviewed changes

nemesifier merged commit 182ebde into openwisp:main Jan 28, 2025
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

shwetd19 commented Jan 23, 2025

pandafy Jan 23, 2025

shwetd19 commented Jan 23, 2025

nemesifier left a comment

nemesifier Jan 24, 2025

shwetd19 commented Jan 24, 2025 •

edited

Loading

nemesifier left a comment •

edited

Loading

shwetd19 commented Jan 26, 2025

shwetd19 commented Jan 27, 2025

nemesifier left a comment

shwetd19 commented Jan 27, 2025

nemesifier left a comment

pandafy left a comment

pandafy Jan 27, 2025

nemesifier Jan 27, 2025

shwetd19 Jan 27, 2025

pandafy Jan 27, 2025

shwetd19 commented Jan 27, 2025

shwetd19 commented Jan 28, 2025

pandafy left a comment

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

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

Conversation

shwetd19 commented Jan 23, 2025

Checklist

Reference to Existing Issue

Description of Changes

pandafy Jan 23, 2025

Choose a reason for hiding this comment

shwetd19 commented Jan 23, 2025

nemesifier left a comment

Choose a reason for hiding this comment

nemesifier Jan 24, 2025

Choose a reason for hiding this comment

shwetd19 commented Jan 24, 2025 • edited Loading

nemesifier left a comment • edited Loading

Choose a reason for hiding this comment

shwetd19 commented Jan 26, 2025

shwetd19 commented Jan 27, 2025

nemesifier left a comment

Choose a reason for hiding this comment

shwetd19 commented Jan 27, 2025

nemesifier left a comment

Choose a reason for hiding this comment

pandafy left a comment

Choose a reason for hiding this comment

pandafy Jan 27, 2025

Choose a reason for hiding this comment

nemesifier Jan 27, 2025

Choose a reason for hiding this comment

shwetd19 Jan 27, 2025

Choose a reason for hiding this comment

pandafy Jan 27, 2025

Choose a reason for hiding this comment

shwetd19 commented Jan 27, 2025

shwetd19 commented Jan 28, 2025

pandafy left a comment

Choose a reason for hiding this comment

shwetd19 commented Jan 24, 2025 •

edited

Loading

nemesifier left a comment •

edited

Loading