Skip to content

Conversation

TahminaAhmed18
Copy link

Summary
This pull request addresses a technical debt issue in BasicCrawler, where request locks were being manually released using deleteRequestLock(). According to the discussion in #2840, this behavior was not aligned with the intended architectural design, as request lifecycle responsibilities should be handled exclusively by markRequestHandled().

Changes Introduced

  • Removed the manual call to source.client.deleteRequestLock(request.id!) from the finally block in BasicCrawler.
  • Simplified the control flow and avoided potential redundancy or race conditions in request unlocking.
  • Added an inline comment referencing the decision to defer unlocking to markRequestHandled().

Related Information

  • Fixes: #2840
  • This change improves architectural consistency by ensuring BasicCrawler does not directly manage request locks, adhering to single-responsibility principles.

@janbuchar
Copy link
Contributor

Hi @TahminaAhmed18 and thanks for your contribution! I'm afraid we cannot merge this in the current state. We would need comprehensive testing to make sure that there is no scenario where BasicCrawler leaves hanging locks.

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.

Remove unlocking requests from BasicCrawler
2 participants