search

jotes / django-cookies-samesite

This repository contains a middleware which automatically sets SameSite attribute for session and csrf cookies in legacy versions of Django.
BSD 3-Clause "New" or "Revised" License
49 stars 35 forks source link

setup.py does not declare dependency on ua-parser #25

Closed timmc-edx closed 4 years ago

timmc-edx commented 4 years ago

Description

When upgrading the version of django-cookies-samesite used in https://github.com/edx/edx-platform and running tests, we found that django-cookies-samesite was importing the ua_parser module but not declaring the ua-parser dependency in its setup.py file (although it is in the requirements.txt file).

What I Did

Here's the automated PR that tries to upgrade all of our dependencies: https://github.com/edx/edx-platform/pull/23984/files -- you can see that it found the new django-cookies-samesite but doesn't see that ua-parser is now also required.

Here's a sample error: https://build.testeng.edx.org/job/edx-platform-python-pipeline-pr/17720/testReport/junit/common.djangoapps.student.tests.test_retirement/TestRegisterRetiredUsername/Run_Tests___lms_unit___test_username_close_to_retired_format_active/

>   from ua_parser import user_agent_parser
E   ImportError: No module named 'ua_parser'

../edx-venv-3.5/edx-venv/lib/python3.5/site-packages/django_cookies_samesite/user_agent_checker.py:1: ImportError
timmc-edx commented 4 years ago

I'd be happy to submit a PR, but would you mind pushing master first? It's currently one behind v0.6.0, so I'm hesitant to make a PR against it.

timmc-edx commented 4 years ago

Here's a branch if you'd like to merge it: https://github.com/jotes/django-cookies-samesite/compare/v0.6.0...timmc-edx:setup-requirements

jotes commented 4 years ago

Thanks @timmc-edx, give me a moment.

jotes commented 4 years ago

@timmc-edx can you check if 0.6.1 fixes the problem for you?

timmc-edx commented 4 years ago

It does, thank you!