Skip to content
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

Fix broken link checker #1423

Merged
merged 7 commits into from
May 15, 2022
Merged

Conversation

timobrembeck
Copy link
Member

Short description

Fix some problems in the broken link checker

Proposed changes

  • Fix change of pagination size in broken link checker
  • Fix pagination in broken link checker
  • Show same URLs only once in broken link checker
  • Delete outdated link objects
  • Translate link checker status
  • Fix uncaught TypeError in bulk action script
  • Add chunk size option of 500

Note: I just noticed I forgot to delete old links on the XLIFF import - this I will have to change as well, otherwise outdated links might be shown in the list.

Resolved issues

Fixes: #1405
Fixes: #1413
Fixes: #1422

@timobrembeck timobrembeck requested a review from a team as a code owner May 13, 2022 21:27
@codeclimate
Copy link

codeclimate bot commented May 13, 2022

Code Climate has analyzed commit 7a33db0 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3

The test coverage on the diff in this pull request is 61.8% (50% is the threshold).

This pull request will bring the total coverage in the repository to 73.4% (0.0% change).

View more on Code Climate.

Copy link
Member

@svenseeberg svenseeberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I cannot see any immediate problems with your code changes. However, I have some more general comments and I stumbled upon one stack trace while updating a broken link with 3 occurrences but I'm not sure how to reproduce:

May 14 12:19:39 ERROR django.request - Internal Server Error: /augsburg/analytics/linkcheck/invalid/3/
Traceback (most recent call last):
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/db/backends/base/base.py", line 242, in _commit
    return self.connection.commit()
psycopg2.errors.ForeignKeyViolation: update or delete on table "linkcheck_url" violates foreign key constraint "linkcheck_link_url_id_c5f791e9_fk" on table "linkcheck_link"
DETAIL:  Key (id)=(1) is still referenced from table "linkcheck_link".


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/views/generic/base.py", line 70, in view
    return self.dispatch(request, *args, **kwargs)
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/views/generic/base.py", line 98, in dispatch
    return handler(request, *args, **kwargs)
  File "/home/sven/Code/integreat-cms-django/integreat_cms/cms/views/linkcheck/linkcheck.py", line 161, in post
    translation.links.all().delete()
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/db/models/query.py", line 746, in delete
    deleted, _rows_count = collector.delete()
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/db/models/deletion.py", line 435, in delete
    signals.post_delete.send(
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/db/transaction.py", line 246, in __exit__
    connection.commit()
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/utils/asyncio.py", line 33, in inner
    return func(*args, **kwargs)
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/db/backends/base/base.py", line 266, in commit
    self._commit()
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/db/backends/base/base.py", line 242, in _commit
    return self.connection.commit()
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/db/utils.py", line 90, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/sven/Code/integreat-cms-django/.venv/lib64/python3.9/site-packages/django/db/backends/base/base.py", line 242, in _commit
    return self.connection.commit()
django.db.utils.IntegrityError: update or delete on table "linkcheck_url" violates foreign key constraint "linkcheck_link_url_id_c5f791e9_fk" on table "linkcheck_link"
DETAIL:  Key (id)=(1) is still referenced from table "linkcheck_link".

integreat_cms/locale/de/LC_MESSAGES/django.po Show resolved Hide resolved
integreat_cms/cms/views/linkcheck/linkcheck.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@timobrembeck timobrembeck force-pushed the bugfix/broken-link-checker-pagination branch from 4a908fa to 8c5bb22 Compare May 14, 2022 20:57
integreat_cms/xliff/utils.py Show resolved Hide resolved
integreat_cms/xliff/utils.py Show resolved Hide resolved
@timobrembeck
Copy link
Member Author

However, I have some more general comments and I stumbled upon one stack trace while playing updating a broken link with 3 occurrences but I'm not sure how to reproduce

I think this might have been caused by a race condition - the post_save signal is called asynchronously in a separate thread - when old links are deleted while the post_save signal is running, the database integrity seems to be violated.
I think I solved this by acquiring linkcheck's internal lock for this - could you play around with it again and see whether you observe any errors?

@svenseeberg
Copy link
Member

svenseeberg commented May 15, 2022

I tested a little more and could not find any issues.

@timobrembeck timobrembeck force-pushed the bugfix/broken-link-checker-pagination branch from 8c5bb22 to 7a33db0 Compare May 15, 2022 22:23
@timobrembeck timobrembeck merged commit 2a85719 into develop May 15, 2022
@timobrembeck timobrembeck deleted the bugfix/broken-link-checker-pagination branch May 15, 2022 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants