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

SQS: Fixes #2091 queue_delete() method doesn't actually delete the queue #2099

Conversation

ivanprjcts
Copy link
Contributor

@ivanprjcts ivanprjcts commented Aug 24, 2024

Fix for issue #2091

.queue_delete() method of SQS channel is not actually deleting the queue. See implementation here.

    def _delete(self, queue, *args, **kwargs):
        """Delete queue by name."""
        if self.predefined_queues:
            return
        super()._delete(queue)
        self._queue_cache.pop(queue, None)
  • I managed to call delete_queue using sqs client (boto3).
  • I wrapped logic to resolve queue URL into a reusable method _resolve_queue_url method for reusability.
  • I added unit tests coverage for new changes.

I took the liberty of creating a new method (_resolve_queue_url). I found it convenient to delegate the responsibility of resolving the queue URL to a separate method.

Thank you :)

ivanprjcts and others added 3 commits August 24, 2024 00:32
* Create new `_resolve_queue_url` method for reusability.

`_new_queue` method retrieves the queue URL from cache
or fetches it from SQS, updating the cache if necessary.

`_delete` method needs to resolve the queue URL in order to
delete the queue from SQS (next commit).

Both methods will be able to reuse the same functionality by
calling the `_resolve_queue_url` method.

* Introduce DoesNotExistQueueException for easier error handling.

`_new_queue` method is responsible for creating a new queue when
 it doesn't exist, utilizing the new exception for clarity.

* Unit test coverage for `_resolve_queue_url` method.
* Add call to `delete_queue` using sqs client.

`_delete` method is expected to delete the specified queue when
called. Previously, this functionality was missing, which has
now been corrected.

The method raises a `DoesNotExistQueueException` if the specified
queue name doesn’t exist.

* Update unit tests with new assertion and mock to verify queue deletion.
@ivanprjcts ivanprjcts marked this pull request as ready for review August 24, 2024 20:23
@auvipy auvipy self-requested a review August 25, 2024 06:43
@thedrow thedrow merged commit a62e613 into celery:main Aug 25, 2024
16 checks passed
@ivanprjcts ivanprjcts deleted the bugfix/sqs-transport-queue_delete-method-not-really-delete-queue branch August 25, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant