Skip to content

Remove prefetched tags in FindingViewSet #13568

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 30, 2025
Merged

Remove prefetched tags in FindingViewSet #13568

merged 1 commit into from
Oct 30, 2025

Conversation

@Maffooch
Copy link
Contributor

@Maffooch Maffooch commented Oct 29, 2025 •
edited
Loading

Eliminate prefetched tags to prevent issues with celery delegation during finding closure operations.

[29/Oct/2025 17:45:00] DEBUG [dojo.decorators:76] dojo_async_task <@task: dojo.notifications.helper.send_mail_notification of dojo at 0x7ffffd054560>: running task in the background as user has not set block_execution to True for admin
[29/Oct/2025 17:45:00] ERROR [dojo.api_v2.exception_handler:49] Can't pickle <class 'dojo.models.Tagulous_Finding_tags'>: attribute lookup Tagulous_Finding_tags on dojo.models failed
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/kombu/serialization.py", line 41, in _reraise_errors
    yield
  File "/usr/local/lib/python3.12/site-packages/kombu/serialization.py", line 220, in dumps
    payload = encoder(data)
              ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/kombu/serialization.py", line 347, in pickle_dumps
    return dumper(obj, protocol=pickle_protocol)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_pickle.PicklingError: Can't pickle <class 'dojo.models.Tagulous_Finding_tags'>: attribute lookup Tagulous_Finding_tags on dojo.models failed

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/rest_framework/views.py", line 512, in dispatch
    response = handler(request, *args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/dojo/api_v2/views.py", line 937, in close
    finding_helper.close_finding(
  File "/app/dojo/finding/helper.py", line 822, in close_finding
    create_notification(
  File "/app/dojo/notifications/helper.py", line 81, in create_notification
    notification_manager_class().create_notification(
  File "/app/dojo/notifications/helper.py", line 636, in create_notification
    self._process_notifications(
  File "/app/dojo/notifications/helper.py", line 855, in _process_notifications
    self._get_manager_instance("mail").send_mail_notification(
  File "/app/dojo/decorators.py", line 103, in __wrapper__
    return func.apply_async(args=args, kwargs=kwargs, countdown=countdown)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/celery/app/task.py", line 601, in apply_async
    return app.send_task(
           ^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/celery/app/base.py", line 930, in send_task
    amqp.send_task_message(P, name, message, **options)
  File "/usr/local/lib/python3.12/site-packages/celery/app/amqp.py", line 523, in send_task_message
    ret = producer.publish(
          ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/kombu/messaging.py", line 178, in publish
    body, content_type, content_encoding = self._prepare(
                                           ^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/kombu/messaging.py", line 280, in _prepare
    body) = dumps(body, serializer=serializer)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/kombu/serialization.py", line 219, in dumps
    with _reraise_errors(EncodeError):
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/contextlib.py", line 158, in __exit__
    self.gen.throw(value)
  File "/usr/local/lib/python3.12/site-packages/kombu/serialization.py", line 45, in _reraise_errors
    reraise(wrapper, wrapper(exc), sys.exc_info()[2])
  File "/usr/local/lib/python3.12/site-packages/kombu/exceptions.py", line 34, in reraise
    raise value.with_traceback(tb)
  File "/usr/local/lib/python3.12/site-packages/kombu/serialization.py", line 41, in _reraise_errors
    yield
  File "/usr/local/lib/python3.12/site-packages/kombu/serialization.py", line 220, in dumps
    payload = encoder(data)
              ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/kombu/serialization.py", line 347, in pickle_dumps
    return dumper(obj, protocol=pickle_protocol)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
kombu.exceptions.EncodeError: Can't pickle <class 'dojo.models.Tagulous_Finding_tags'>: attribute lookup Tagulous_Finding_tags on dojo.models failed

[sc-11996]

@Maffooch Maffooch requested a review from mtesauro as a code owner October 29, 2025 22:57
@github-actions github-actions bot added the apiv2 label Oct 29, 2025
mtesauro
mtesauro approved these changes Oct 30, 2025
View reviewed changes
Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

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

Approved

valentijnscholten
valentijnscholten reviewed Oct 30, 2025
View reviewed changes
dojo/api_v2/views.py
context={"request": request},
)
if finding_close.is_valid():
# Remove the prefetched tags to avoid issues with delegating to celery
Copy link
Member

@valentijnscholten valentijnscholten Oct 30, 2025

Choose a reason for hiding this comment

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

what kind of issues and where?

Copy link
Contributor Author

@Maffooch Maffooch Oct 30, 2025

Choose a reason for hiding this comment

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

Updated the description with the stack trace

Copy link
Member

@valentijnscholten valentijnscholten Oct 30, 2025

Choose a reason for hiding this comment

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

Thanks. Almost like we would be better of by just having a couple of M2M fields named tags and some helper methods :-)

Copy link
Member

@valentijnscholten valentijnscholten Oct 30, 2025

Choose a reason for hiding this comment

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

ultimately I think it might be better to only send the finding.id to a celery task and let it fetch the latest and greatest data from the db. but for now I understand this move.

@valentijnscholten valentijnscholten added this to the 2.52.0 milestone Oct 30, 2025
valentijnscholten
valentijnscholten approved these changes Oct 30, 2025
View reviewed changes
dojo/api_v2/views.py
context={"request": request},
)
if finding_close.is_valid():
# Remove the prefetched tags to avoid issues with delegating to celery
Copy link
Member

@valentijnscholten valentijnscholten Oct 30, 2025

Choose a reason for hiding this comment

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

ultimately I think it might be better to only send the finding.id to a celery task and let it fetch the latest and greatest data from the db. but for now I understand this move.

@mtesauro mtesauro merged commit 09f7ffb into bugfix Oct 30, 2025
150 checks passed
@Maffooch
Copy link
Contributor Author

Maffooch commented Oct 30, 2025

Yeah, I agree that it would be ideal to only pass the ID and get the object from the DB in the task. The unfortunate part of the task is that it can accept a product, engagement, test, finding, or note, so it would take quite a restructure to accommodate that model

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mtesauro mtesauro mtesauro approved these changes

@valentijnscholten valentijnscholten valentijnscholten approved these changes

@dogboat dogboat dogboat approved these changes

@Jino-T Jino-T Jino-T approved these changes

@paulOsinski paulOsinski Awaiting requested review from paulOsinski

Assignees

No one assigned

Labels

apiv2

Projects

None yet

Milestone

2.52.0

Development

Successfully merging this pull request may close these issues.

6 participants

@Maffooch @mtesauro @valentijnscholten @dogboat @Jino-T