8000 Readme update to add settings.py params required to install and use the app by nicktgr15 · Pull Request #24 · jazzband/django-cookie-consent · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Readme update to add settings.py params required to install and use the app #24

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
Closed
wants to merge 2 commits into from
Closed

Readme update to add settings.py params required to install and use the app #24

wants to merge 2 commits into from

Conversation

nicktgr15
Copy link

No description provided.

@bmihelac
Copy link
Collaborator

@nicktgr15 this two settings should already have defaults as set in https://github.com/bmihelac/django-cookie-consent/blob/master/cookie_consent/conf.py#L10-L11

Does it not work for you without this settings?

@JesseDeLoore
Copy link

FYI: COOKIE_CONSENT_NAME with a bytes value breaks on modern installations.
So it doesn't work without setting this explicitly in the settings.py
No complaints on COOKIE_CONSENT_MAX_AGE

@motasay
Copy link
motasay commented Apr 9, 2021

@bmihelac Same as what @JesseDeLoore said. I'm getting TypeError: cannot use a string pattern on a bytes-like object on Django==3.1.7 until I set the name as a string in settings.

web_1  |   File "/usr/local/lib/python3.7/site-packages/cookie_consent/views.py", line 53, in post
web_1  |     self.process(request, response, varname)
web_1  |   File "/usr/local/lib/python3.7/site-packages/cookie_consent/views.py", line 63, in process
web_1  |     accept_cookies(request, response, varname)
web_1  |   File "/usr/local/lib/python3.7/site-packages/cookie_consent/util.py", line 98, in accept_cookies
web_1  |     set_cookie_dict_to_response(response, cookie_dic)
web_1  |   File "/usr/local/lib/python3.7/site-packages/cookie_consent/util.py", line 43, in set_cookie_dict_to_response
web_1  |     settings.COOKIE_CONSENT_MAX_AGE)
web_1  |   File "/usr/local/lib/python3.7/site-packages/django/http/response.py", line 169, in set_cookie
web_1  |     self.cookies[key] = value
web_1  |   File "/usr/local/lib/python3.7/http/cookies.py", line 495, in __setitem__
web_1  |     self.__set(key, rval, cval)
web_1  |   File "/usr/local/lib/python3.7/http/cookies.py", line 485, in __set
web_1  |     M.set(key, real_value, coded_value)
web_1  |   File "/usr/local/lib/python3.7/http/cookies.py", line 351, in set
web_1  |     if not _is_legal_key(key):
web_1  | TypeError: cannot use a string pattern on a bytes-like object

@bmihelac
Copy link
Collaborator

I understand, it looks like that default value should be string and not bytestring.
Like removing b here: https://github.com/bmihelac/django-cookie-consent/blob/master/cookie_consent/conf.py#L10

@some1ataplace some1ataplace mentioned this pull request Mar 7, 2022
Closed
@some1ataplace
Copy link
Contributor

@JesseDeLoore and @motasay Can you tell us how we could recreate this error ourselves?

@JesseDeLoore
Copy link

@JesseDeLoore and @motasay Can you tell us how we could recreate this error ourselves?

The error happened when I didn't put a value for COOKIE_CONSENT_NAME and the default with a bytes value was used.
Since this issue is a little older now maybe it is resolved on the python side.

As a point of order: is there a specific reason the default value uses a byte-string ?

@some1ataplace
Copy link
Contributor
some1ataplace commented Mar 13, 2022 •
edited
Loading

@JesseDeLoore Thank you. This error still occurs if you do not set a value for COOKIE_CONSENT_NAME in your own settings.py and the default is used instead.

@nicktgr15 nicktgr15 changed the title Readme update to add settings.py params required to instal and use the app Readme update to add settings.py params required to install and use the app Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nicktgr15 @bmihelac @JesseDeLoore @motasay @some1ataplace
0