Regex support added for ignore URL list in settings.py #116
Conversation
|
Can someone please review the installation failure for Django 5.0 |
|
@ingvaldlorentzen @JonasKs @nschlemm Can you please review workflows for Django Version 5.0 above. |
|
Thanks for the PR! The code needs tests, and I'd like to use a |
|
@JonasKs Thanks for your recommendations. Followed this - The code needs tests, and I'd like to use a re.compile solution instead of this. Inspiration can be found here Updates:
Please have a look on the Django 5.0 workflows. |
|
@JonasKs @nschlemm @ingvaldlorentzen Can you please review the PR. |
|
I'll get to it when I have time. I won't have time until next week at earliest. |
|
@JonasKs No issues. Check this as per your convenience. Thanks 👍 |
|
Wow, I definitely didn't intend to just close this out of nowhere. I was looking for this now, and couldn't find it. @sauravsharma1998 , I'm sorry. |
|
The implementation would be a breaking change - since you change the current |
| IGNORE_URLS = [] | ||
| for url in settings.ignore_urls: | ||
| url_regex = url.replace('*', r'[\s\S]*') # noqa | ||
| url_regex = '^' + url_regex + '$' |
There was a problem hiding this comment.
If we make this a new setting, e.g. ignore_url_regex, we can also skip this.
There was a problem hiding this comment.
@JonasKs Thanks for the suggestions. I have moved it to a new settings. But facing errors in CI pipeline.
There was a problem hiding this comment.
I’m on vacation, but I’ll check when I’m back. 😊
|
There's failing pipelines, even though I haven't changed anything and I've deleted all caches.. I'll have to look further into this, but I'm not sure what's happening. |
| for url in settings.ignore_regex_urls: | ||
| url_regex = url.replace('*', r'[\s\S]*') # noqa | ||
| url_regex = '^' + url_regex + '$' | ||
| IGNORE_URLS.append(re.compile(url_regex)) |
There was a problem hiding this comment.
This will compile every regex twice every requests. It seems like a waste of processing power, and we will not get the benefits of compiled regex since they will bê compiled every time they are needed.
I wonder if it is possible to compile tem just once, maybe in the middleware setup or at the ignore_regex_urls settings.
| url_regex = url.replace('*', r'[\s\S]*') # noqa | ||
| url_regex = '^' + url_regex + '$' | ||
| IGNORE_URLS.append(re.compile(url_regex)) | ||
| return any(url.match(endpoint) for url in IGNORE_URLS) |
There was a problem hiding this comment.
| return any(url.match(endpoint) for url in IGNORE_URLS) | |
| return any(url.search(endpoint) for url in IGNORE_URLS) |
Using search would remove the needs to change the user's regex a few lines above, and would make it compatible with the way django itself handle this kind of settings, like SECURE_REDIRECT_EXEMPT. Using search also should even allow to use a single settings.
Regex Support Added for IGNORE_URLS:
admin/* [All URL pattern with starting with admin/ will be ignored for GUID generation]
Suggestion Link: #104