Skip to content

Conversation

@skissane-medallia
Copy link

High-level summary

Need to upgrade this to use the latest version of six to solve a dependency issue.

I tried to run the tests to confirm it worked with the new version of six, found several issues, fixed them. Getting them to run required further dependency upgrades. In the process also discovered some documentation regressions (changes made in past without updating documentation), fixed them too.

Replaced Travis CI with GitHub Actions to get CI for the changes.

Detailed change list

  • setup.py
    • Upgrade required version of six from 1.10.0 to 1.16.0
    • Specify Django must be 3.0.x, 3.1.x or 3.2.x (this code doesn't work with Django 4+)
    • Description: make clear supports Django 3.x not just Django 3.0.x
    • Remove mention of supporting Python 2: newer dependency versions don't support Python 2 hence support for it has to be dropped from this package, and Python 2 has not been supported for over 4 years now anyway
    • tests_require: nose is no longer maintained and not compatible with Python 3.12. Replace with pynose fork
  • dev-requirements.txt
    • Upgrade sphinx, wheel, setuptools: existing versions did not work with Python 3.12
  • docs/api.rst and docs/request-objects.rst
    • django_wsgi.handler.APPLICATION had been replaced with django_wsgi.handler.get_wsgi_application() in commit 1aacf23 (March 2020) but these documentation pages were not updated accordingly
  • tests/dummy_django_project/urls.py
    • This was using django.conf.urls.patterns which does not exist anymore in Django 3.x. Replaced with use of django.conf.urls.re_path
  • tests/requirements.txt
    • Upgrade coverage, webOb, coveralls for Python 3.12 compatibility
    • Replace nose with pynose (see setup.py above for why)
  • tests/test_handler.py
    • This test was broken by the APPLICATION => get_wsgi_application() change mentioned above, fix it
  • .gitignore
    • Add __pycache__, .coverage, .venv
  • tests/test_embedded_wsgi.py
    • test_closure_response was failing because it was wrongly asserting on app.app_iter.closed, which is actually the request closure status not the response closure status. Changed it to assert on django_response.closed instead
    • test_original_environ_not_modified was failing due to newer WebOb version adding CONTENT_LENGTH and webob.is_body_seekable to the request environment. Given the point of the test is to check that call_wsgi_app() isn't changing the request environment, the check is just move the capturing of the environment for comparison to after WebOb has set up the request
    • Both test_headers_are_copied_over and test_right_path were broken in Django 3.2.x (but not 3.1.x). This is because the undocumented response._headers in 3.1.x is replaced by response.headers in 3.2.x, which also has a different format. To make the tests work with either Django version, added a new method to tests/__init__.py called get_response_headers which hides the difference. (Note the reading of the response headers only happens in the tests, not the runtime code, so no code change is needed to the runtime code to handle this issue.)
  • .travis.yml
    • replaced with GitHub Actions workflow .github/workflows/run-tests.yaml: Travis is no longer (since Decemeber 2020) free for open source projects, but GitHub Actions still is
    • Used this tool to do the conversion: https://akx.github.io/travis-to-github-actions/
    • Used ubuntu-latest runner: output of above tool uses ubuntu-18.04, but GitHub no longer supports it, hence there are no available runners for it, hence the job hangs forever before starting
    • Remove testing of unsupported CPython versions (2.7, 3.3, 3.4)
    • Test all currently supported CPython versions (3.8 thru 3.12)
    • Test PyPy 3.9/3.10
    • Use latest action versions: checkout@v4, setup-python@v5
    • Temporarily comment out Coveralls (will put it back once API key sorted out)

High-level summary
------------------

Need to upgrade this to use the latest version of `six` to solve a dependency issue.

I tried to run the tests to confirm it worked with the new version of `six`, found several issues, fixed them. Getting them
to run required further dependency upgrades. In the process also discovered some documentation regressions (changes made in
past without updating documentation), fixed them too.

Replaced Travis CI with GitHub Actions to get CI for the changes.

Detailed change list
--------------------

- `setup.py`
  - Upgrade required version of six from 1.10.0 to 1.16.0
  - Specify Django must be 3.0.x, 3.1.x or 3.2.x (this code doesn't work with Django 4+)
  - Description: make clear supports Django 3.x not just Django 3.0.x
  - Remove mention of supporting Python 2: newer dependency versions don't support Python 2 hence support for it has to
    be dropped from this package, and Python 2 has not been supported for over 4 years now anyway
  - `tests_require`: `nose` is no longer maintained and not compatible with Python 3.12. Replace with `pynose` fork
- `dev-requirements.txt`
  - Upgrade sphinx, wheel, setuptools: existing versions did not work with Python 3.12
- `docs/api.rst` and `docs/request-objects.rst`
  - `django_wsgi.handler.APPLICATION` had been replaced with `django_wsgi.handler.get_wsgi_application()`
    in commit 1aacf23 (March 2020) but these documentation pages were not
    updated accordingly
- `tests/dummy_django_project/urls.py`
  - This was using `django.conf.urls.patterns` which does not exist anymore in Django 3.x.
    Replaced with use of `django.conf.urls.re_path`
- `tests/requirements.txt`
  - Upgrade `coverage`, `webOb`, `coveralls` for Python 3.12 compatibility
  - Replace `nose` with `pynose` (see `setup.py` above for why)
- `tests/test_handler.py`
  - This test was broken by the `APPLICATION` => `get_wsgi_application()` change mentioned above, fix it
- `.gitignore`
  - Add `__pycache__`, `.coverage`, `.venv`
- `tests/test_embedded_wsgi.py`
  - `test_closure_response` was failing because it was wrongly asserting on `app.app_iter.closed`,
    which is actually the request closure status not the response closure status. Changed it to assert on
    `django_response.closed` instead
  - `test_original_environ_not_modified` was failing due to newer WebOb version adding `CONTENT_LENGTH` and
    `webob.is_body_seekable` to the request environment. Given the point of the test is to check that
    `call_wsgi_app()` isn't changing the request environment, the check is just move the capturing of the
    environment for comparison to after WebOb has set up the request
  - Both `test_headers_are_copied_over` and `test_right_path` were broken in Django 3.2.x (but not 3.1.x).
    This is because the undocumented `response._headers` in 3.1.x is replaced by `response.headers` in 3.2.x,
    which also has a different format. To make the tests work with either Django version, added a new method
    to `tests/__init__.py` called `get_response_headers` which hides the difference. (Note the reading of
    the response headers only happens in the tests, not the runtime code, so no code change is needed to
    the runtime code to handle this issue.)
- `.travis.yml`
  - replaced with GitHub Actions workflow `.github/workflows/run-tests.yaml`: Travis is no longer (since
    Decemeber 2020) free for open source projects, but GitHub Actions still is
  - Used this tool to do the conversion: https://akx.github.io/travis-to-github-actions/
  - Used `ubuntu-latest` runner: output of above tool uses `ubuntu-18.04`, but GitHub no longer supports it, hence there
    are no available runners for it, hence the job hangs forever before starting
  - Remove testing of unsupported CPython versions (2.7, 3.3, 3.4)
  - Test all currently supported CPython versions (3.8 thru 3.12)
  - Test PyPy 3.9/3.10
  - Use latest action versions: checkout@v4, setup-python@v5
  - Temporarily comment out Coveralls (will put it back once API key sorted out)
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.

1 participant