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.

Sign up for GitHub

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

Jump to bottom

Running setup.py directly is deprecated #737

Merged
merged 9 commits into from
Jul 29, 2024
Merged

Running setup.py directly is deprecated #737

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
672a395
Running setup.py directly is deprecated
cclauss Feb 22, 2024
38bc4eb
Makefile
cclauss Feb 27, 2024
fe2850a
Apply suggestions from code review
cclauss Feb 27, 2024
933eefb
Update Makefile
cclauss Feb 28, 2024
17f53a6
Update docker/base/Dockerfile
cclauss Feb 28, 2024
bc4684b
Dockerfile: RUN pip3 install --user --editable ".[develop]"
cclauss Feb 29, 2024
20f37b5
docker-compose.yml: tags: - django-celery-beat_base:latest
cclauss Jul 8, 2024
c864ba5
Merge branch 'main' into running-setup.py-directly-is-deprecated
cclauss Jul 29, 2024
f3cd270
python -m pip install --upgrade build twine
cclauss Jul 29, 2024
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
8 changes: 5 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ help:
@echo " clean ------------ - Non-destructive clean"
@echo " clean-pyc - Remove .pyc/__pycache__ files"
@echo " clean-docs - Remove documentation build artifacts."
@echo " clean-build - Remove setup artifacts."
@echo " clean-build - Remove build artifacts."
@echo "bump - Bump patch version number."
@echo "bump-minor - Bump minor version number."
@echo "bump-major - Bump major version number."
Expand All @@ -65,7 +65,9 @@ bump-major:
bumpversion major

release:
python setup.py register sdist bdist_wheel upload --sign --identity="$(PGPIDENT)"
python -m pip install --upgrade build setuptools wheel twine
cclauss marked this conversation as resolved.
Show resolved Hide resolved
python -m build
twine upload --sign --identity="$(PGPIDENT) dist/*"

Documentation:
(cd "$(SPHINX_DIR)"; $(MAKE) html)
Expand Down Expand Up @@ -145,7 +147,7 @@ cov:
(cd $(TESTDIR); $(PYTEST) -x --cov="$(PROJ)" --cov-report=html)

build:
$(PYTHON) setup.py sdist bdist_wheel
$(PYTHON) -m build

distcheck: lint test clean

Expand Down
13 changes: 6 additions & 7 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ To install using ``pip``:

.. code-block:: bash

$ pip install -U django-celery-beat
$ pip install --upgrade django-celery-beat

Downloading and installing from source
--------------------------------------
Expand All @@ -264,14 +264,13 @@ You can install it by doing the following :

.. code-block:: bash

$ python3 -m venv .venv
Copy link

Choose a reason for hiding this comment

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

I'm not sure venv management should be covered here. There are tons of ways of to manage a venv, and forcing one in a tutorial ends up badly in my experience. Just a mention of a venv should be enough, as is before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is Python's built-in way of creating and enabling a venv. Users are free to choose alternative methods but I prefer explicit steps here to document what we know works and avoid issues being opened around implicit steps.

Copy link

@ulgens ulgens Feb 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I prefer explicit steps here to document

This can be easily paraphrased as "pushing an opinionated tooling", which worries me. IMO, for anything related to venv, considering how every detail of that topic is open for a million different debates, it's better to keep an equal distance to all options.

Of course, you can ignore my comment; it's not a big thing, but I believe these types of details are more important than most people care for.

Copy link
Contributor Author

@cclauss cclauss Feb 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Opinionated tooling would have been a suggestion to use uv. 😄

I do tons of responding to Node.js and node-gyp issues. It is there (and elsewhere) that I have learned the hard way that incomplete directions and assumptions lead to long-winded discussions.

it's better to keep an equal distance to all options.

I think suggesting the one that is built-in and requires zero third-party software is relatively safe.

$ source .venv/bin/activate
$ pip install --upgrade build pip
$ tar xvfz django-celery-beat-0.0.0.tar.gz
$ cd django-celery-beat-0.0.0
$ python setup.py build
# python setup.py install

The last command must be executed as a privileged user if
you are not currently using a virtualenv.

$ python -m build
$ pip install --upgrade .

After installation, add ``django_celery_beat`` to Django's settings module:

Expand Down
2 changes: 2 additions & 0 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ services:
build:
context: .
dockerfile: docker/base/Dockerfile
tags:
- django-celery-beat_base:latest
command: ["sleep", "inf"]

django:
Expand Down
4 changes: 2 additions & 2 deletions docker/base/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ RUN apt-get update && apt-get install --yes --no-install-recommends \
wait-for-it

RUN pip3 install --user \
build \
django-createsuperuserwithpassword \
psycopg2-binary

Expand All @@ -19,8 +20,7 @@ COPY requirements/ /app/requirements/

WORKDIR /app

RUN python3 setup.py develop --user

RUN pip3 install --user --editable ".[develop]"

WORKDIR /

Expand Down
20 changes: 14 additions & 6 deletions docs/includes/installation.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@ or from source.

To install using `pip`,::

$ pip install -U django-celery-beat
$ pip install --upgrade django-celery-beat

Installing the current default branch
-------------------------------------

$ python3 -m venv .venv
Copy link

Choose a reason for hiding this comment

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

Same here: #737 (comment) With these changes, it looks like the steps are more about venvs, not the actual goal of the doc.

$ source .venv/bin/activate
$ pip install --upgrade pip
$ pip install git+https://github.com/celery/django-celery-beat.git

Downloading and installing from source
--------------------------------------
Expand All @@ -16,13 +24,13 @@ http://pypi.python.org/pypi/django-celery-beat

You can install it by doing the following,::

$ python3 -m venv .venv
$ source .venv/bin/activate
$ pip install --upgrade build pip
$ tar xvfz django-celery-beat-0.0.0.tar.gz
$ cd django-celery-beat-0.0.0
$ python setup.py build
# python setup.py install

The last command must be executed as a privileged user if
you are not currently using a virtualenv.
$ python -m build
$ pip install .

Using the development version
-----------------------------
Expand Down
2 changes: 1 addition & 1 deletion t/unit/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import pytest
# we have to import the pytest plugin fixtures here,
# in case user did not do the `python setup.py develop` yet,
# in case user did not yet `pip install ".[develop]"`,
Copy link

Choose a reason for hiding this comment

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

Doesn't it make better sense with "yet" at end of the sentence, as the before?

Copy link
Contributor Author

@cclauss cclauss Feb 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It felt like a dangling participle when I read it. (But I would bet dollars to doughnuts that it is not a participle. 😃)

# that installs the pytest plugin into the setuptools registry.
from celery.contrib.pytest import (celery_app, celery_config,
celery_enable_logging, celery_parameters,
Expand Down