Skip to content

Perform redirects at db level #6779

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.

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

Closed
wants to merge 2 commits into from
Closed

Conversation

humitos
Copy link
Member

@humitos humitos commented Mar 14, 2020

This is another attempt to perform redirects at DB level and be able to put them at Proxito level, allowing to redirect to be executed not only on 404s.

It continues the work we did at #6398 but that we reverted at #6430 because we found an issue with Exact Redirects with from_url smaller than 5 chars long. This PR workarounds that case, giving us the opportunity to keep exploring this idea of performing redirects at DB level.

humitos added 2 commits March 13, 2020 19:06
This commit reverts 88c7640 from PR #6430

We found a problem with `from_url` that are smaller than 5 char
length. I'm reverting this here and adding a new commit that will
workaround this problem.
We first perform a `.count()` query to check if the DB SQL query will
fail. If it fails, we ignore the `exact` Q object and we continue with
the redirects check ignoring all the Exact Redirects for this project.

This query will *only fail* if there is at least one Exact Redirect
with less than 5 chars long on this project. This is because we are
substracting the `$rest` part from the `from_url` field and we can't
end up with a negative number.
@humitos
Copy link
Member Author

humitos commented Mar 14, 2020

I checked how many of these redirects we have:

  • .org: 51
  • .com: 0

Most of them are / and '' which do not work anyways. The rest may be worth to double check also, because they would only work if the project is single version since there is not enough chars to create a valid redirect with less than 5 chars in a multi-version project (/en/ is 4 chars already)

In [26]: Counter(list(Redirect.objects.annotate(from_url_length=ExpressionWrapper(Length('from_url'), output_field=IntegerField())).filter(from_url_length__lt=5).filter(redirect_type='exact'
    ...: ).values_list('from_url', flat=True)))
Out[26]: 
Counter({'': 7,
         '/': 25,
         '/ko': 1,
         '/en': 1,
         '/who': 1,
         '/why': 1,
         '/faq': 2,
         '/bar': 1,
         '/foo': 1,
         '/rus': 1,
         '/soc': 1,
         '*': 2,
         '?q': 1,
         '/en/': 3,
         '3.1': 1,
         '/ko/': 2})

I'd say that it worth to have a talk around this and discuss if it's doable, but it seems we are pretty close to this, IMO.

@humitos
Copy link
Member Author

humitos commented Mar 19, 2020

Closing in favor of #6780

@humitos humitos closed this Mar 19, 2020
@stsewd stsewd deleted the humitos/redirects-at-db-level branch March 19, 2020 18:40
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