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 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions files/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Core dependencies
attrs
service_identity

# Django and related packages
django-redis~=5.4.0
django-pipeline~=3.1.0
django-cors-headers~=4.4.0
channels_redis~=4.2.0


Copy link
Member

Choose a reason for hiding this comment

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

Remove this extra blank line

# WSGI server
uwsgi

# Monitoring and error reporting
raven

# Email backend
django-celery-email

# Remove deprecated packages
jsonfield2==*; python_version < "0.0" # Ensures this is never installed
Comment on lines +20 to +21
Copy link
Member

Choose a reason for hiding this comment

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

Will this remove the package if it was previously installed?

@nemesifier can we remove the task which ensures that this package is removed?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes we can

154 changes: 46 additions & 108 deletions tasks/pip.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,27 @@
- pip
- setuptools
- wheel
- attrs
state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
retries: 5
delay: 10
register: result
until: result is success

- name: Remove jsonfield2
pip:
name:
- jsonfield2
state: absent
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
retries: 2
retries: 3
delay: 5
register: result
until: result is success
notify: Reload application

- name: Install openwisp2 controller and its dependencies
- name: Verify requirements.txt exists
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to verify that requirements.txt exists?

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 removed this part in my latest commit

stat:
path: "{{ role_path }}/files/requirements.txt"
Copy link
Member

Choose a reason for hiding this comment

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

Where is this var role_path defined?

register: req_file

- name: Debug requirements file path
debug:
msg: "Looking for requirements file at {{ role_path }}/files/requirements.txt"
when: req_file.stat is defined

- name: Install Python dependencies from requirements.txt
pip:
name:
- "{{ openwisp2_controller_version }}"
- service_identity
requirements: "{{ role_path }}/files/requirements.txt"
state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
Expand All @@ -43,34 +37,24 @@
delay: 10
register: result
until: result is success
tags:
- molecule-idempotence-notest

- name: Install channels_redis~=4.2.0
- name: Install openwisp2 controller
pip:
name:
- channels_redis~=4.2.0
name: "{{ openwisp2_controller_version }}"
state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
retries: 1
delay: 10
register: result
until: result is success

- name: Install django-redis
pip:
name: "django-redis~=5.4.0"
state: present
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
environment:
LC_CTYPE: "en_US.UTF-8"
notify: Reload application
retries: 5
delay: 10
register: result
until: result is success
tags:
- molecule-idempotence-notest

- name: Install openwisp2 network topology and its dependencies
- name: Install openwisp2 network topology
when: openwisp2_network_topology
pip:
name: "{{ openwisp2_network_topology_version }}"
Expand All @@ -85,7 +69,7 @@
tags:
- molecule-idempotence-notest

- name: Install openwisp firmware upgrader and its dependencies
- name: Install openwisp firmware upgrader
when: openwisp2_firmware_upgrader
pip:
name: "{{ openwisp2_firmware_upgrader_version }}"
Expand All @@ -100,7 +84,7 @@
tags:
- molecule-idempotence-notest

- name: Install openwisp monitoring and its dependencies
- name: Install openwisp monitoring
when: openwisp2_monitoring
pip:
name: "{{ openwisp2_monitoring_version }}"
Expand All @@ -115,7 +99,7 @@
tags:
- molecule-idempotence-notest

- name: Install openwisp2_radius and its dependencies
- name: Install openwisp2_radius
when: openwisp2_radius
pip:
name: "{{ openwisp2_radius_version }}"
Expand All @@ -130,48 +114,23 @@
tags:
- molecule-idempotence-notest

- name: Install django-cors-headers
when: openwisp2_django_cors.get('enabled')
pip:
name: "django-cors-headers~=4.4.0"
state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
retries: 5
delay: 10
register: result
until: result is success
notify: Reload application

- name: Install extra python packages
pip:
name: "{{ openwisp2_extra_python_packages }}"
state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
retries: 5
delay: 10
when: openwisp2_extra_python_packages | length > 0
retries: 3
delay: 5
register: result
until: result is success
notify: Reload application
tags: [extra_pip]

- name: Install static minification dependencies
pip:
name:
- django-pipeline~=3.1.0
state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
retries: 5
delay: 10
register: result
until: result is success
notify: Reload application

- name: Install uwsgi
- name: Install django
pip:
name: uwsgi
name: "{{ openwisp2_django_version }}"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure but can we move Django also in requirements.txt file?
cc @nemesifier

Copy link
Member

Choose a reason for hiding this comment

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

Not for now, because that's configurable.

state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
Expand All @@ -180,69 +139,48 @@
register: result
until: result is success
notify: Reload application
tags:
- molecule-idempotence-notest

- name: Install psycopg2
when: openwisp2_database.engine in ["django.db.backends.postgresql", "django.contrib.gis.db.backends.postgis"]
pip:
name: psycopg2
name: psycopg2-binary
state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
retries: 5
delay: 10
retries: 3
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for changing retries and delays?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is wrong.

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 fixed this part in my latest commit to default retries as 5 for all

delay: 5
register: result
until: result is success
notify: Reload application

- name: Install MySQL-python
when: openwisp2_database.engine in ["django.db.backends.mysql", "django.contrib.gis.db.backends.mysql"]
- name: Install MySQL-python for Python 2
when:
- openwisp2_database.engine in ["django.db.backends.mysql", "django.contrib.gis.db.backends.mysql"]
- openwisp2_python is version('3.0', '<')
pip:
name: MySQL-python
state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
retries: 5
delay: 10
register: result
until: result is success
notify: Reload application

- name: Install raven (sentry client)
when: openwisp2_sentry.get('dsn')
pip:
name: raven
state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
retries: 5
delay: 10
retries: 3
delay: 5
register: result
until: result is success
notify: Reload application
Copy link
Member

Choose a reason for hiding this comment

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

raven is only installed when openwisp2_sentry.get('dsn'), let's remove it from requirements.txt


- name: Install django-celery-email
- name: Install mysqlclient for Python 3
when:
- openwisp2_database.engine in ["django.db.backends.mysql", "django.contrib.gis.db.backends.mysql"]
- openwisp2_python is version('3.0', '>=')
pip:
name: django-celery-email
name: mysqlclient
state: latest
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
when: openwisp2_email_backend == "djcelery_email.backends.CeleryEmailBackend"
Copy link
Member

Choose a reason for hiding this comment

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

As I suggested in the previous review, don't add packages to requirements.txt which are installed conditionally.

Copy link
Member

Choose a reason for hiding this comment

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

we could install it anyway, it shouldn't be a big deal, what do you think?

retries: 5
delay: 10
register: result
until: result is success
notify: Reload application

- name: Install django
pip:
name: "{{ openwisp2_django_version }}"
state: present
virtualenv: "{{ virtualenv_path }}"
virtualenv_python: "{{ openwisp2_python }}"
retries: 5
delay: 10
retries: 3
delay: 5
register: result
until: result is success
notify: Reload application
tags:
- molecule-idempotence-notest
Loading