Project

Profile

Help

Issue #2988

closed

Exception when raising a user-Defined Exception that has a custom __init__ signature

Added by bmbouter over 7 years ago. Updated over 5 years ago.

Status:
CLOSED - WORKSFORME
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
Platform Release:
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Sprint:
Quarter:

Description

Consider this diff applied onto a sync task in core:

[bmbouter@localhost pulp]$ git diff platform/pulpcore/app/tasks/importer.py
diff --git a/platform/pulpcore/app/tasks/importer.py b/platform/pulpcore/app/tasks/importer.py
index 0ef18a8..bab5d76 100644
--- a/platform/pulpcore/app/tasks/importer.py
+++ b/platform/pulpcore/app/tasks/importer.py
@@ -51,6 +51,12 @@ def delete(repo_name, importer_name):
     models.Importer.objects.filter(name=importer_name, repository__name=repo_name).delete()


+class SomeError(Exception):
+    def __init__(self, url):
+        self.url = url
+        super().__init__()
+
+
 @shared_task(base=UserFacingTask)
 def sync(repo_name, importer_name):
     """
@@ -72,4 +78,5 @@ def sync(repo_name, importer_name):

     with storage.working_dir_context() as working_dir:
         importer.working_dir = working_dir
+        raise SomeError('asdf')
         importer.sync()

When you call a sync() task, you'll experience the following exception:

Aug 22 17:22:03 pulp3 celery[22903]: [2017-08-22 17:22:03,336: ERROR/ForkPoolWorker-1] Task failed : [65f60bc5-6433-4dda-a5c0-22b22c82ceb0]
Aug 22 17:22:03 pulp3 celery[22903]: [2017-08-22 17:22:03,342: ERROR/ForkPoolWorker-1] Task pulpcore.app.tasks.importer.sync[65f60bc5-6433-4dda-a5c0-22b22c82ceb0] raised unexpected: SomeError()
Aug 22 17:22:03 pulp3 celery[22903]: Traceback (most recent call last):
Aug 22 17:22:03 pulp3 celery[22903]:   File "/home/vagrant/.virtualenvs/pulp/lib/python3.5/site-packages/celery/app/trace.py", line 374, in trace_task
Aug 22 17:22:03 pulp3 celery[22903]:     R = retval = fun(*args, **kwargs)
Aug 22 17:22:03 pulp3 celery[22903]:   File "/home/vagrant/devel/pulp/platform/pulpcore/tasking/tasks.py", line 272, in __call__
Aug 22 17:22:03 pulp3 celery[22903]:     return super(UserFacingTask, self).__call__(*args, **kwargs)
Aug 22 17:22:03 pulp3 celery[22903]:   File "/home/vagrant/.virtualenvs/pulp/lib/python3.5/site-packages/celery/app/trace.py", line 629, in __protected_call__
Aug 22 17:22:03 pulp3 celery[22903]:     return self.run(*args, **kwargs)
Aug 22 17:22:03 pulp3 celery[22903]:   File "/home/vagrant/devel/pulp/platform/pulpcore/app/tasks/importer.py", line 81, in sync
Aug 22 17:22:03 pulp3 celery[22903]:     raise SomeError('asdf')
Aug 22 17:22:03 pulp3 celery[22903]: pulpcore.app.tasks.importer.SomeError
Aug 22 17:22:03 pulp3 celery[22903]: [2017-08-22 17:22:03,344: ERROR/MainProcess] Task handler raised error: <MaybeEncodingError: Error sending result: ''(1, <ExceptionInfo: SomeError()>, None)''. Reason: ''PicklingError("Can\'t pickle <class \'pulpcore.app.tasks.importer.SomeError\'>: it\'s not the same object as pulpcore.app.tasks.importer.SomeError",)''.>
Aug 22 17:22:03 pulp3 celery[22903]: Traceback (most recent call last):
Aug 22 17:22:03 pulp3 celery[22903]:   File "/home/vagrant/.virtualenvs/pulp/lib/python3.5/site-packages/billiard/pool.py", line 362, in workloop
Aug 22 17:22:03 pulp3 celery[22903]:     put((READY, (job, i, result, inqW_fd)))
Aug 22 17:22:03 pulp3 celery[22903]:   File "/home/vagrant/.virtualenvs/pulp/lib/python3.5/site-packages/billiard/queues.py", line 366, in put
Aug 22 17:22:03 pulp3 celery[22903]:     self.send_payload(ForkingPickler.dumps(obj))
Aug 22 17:22:03 pulp3 celery[22903]:   File "/home/vagrant/.virtualenvs/pulp/lib/python3.5/site-packages/billiard/reduction.py", line 56, in dumps
Aug 22 17:22:03 pulp3 celery[22903]:     cls(buf, protocol).dump(obj)
Aug 22 17:22:03 pulp3 celery[22903]: billiard.pool.MaybeEncodingError: Error sending result: ''(1, <ExceptionInfo: SomeError()>, None)''. Reason: ''PicklingError("Can\'t pickle <class \'pulpcore.app.tasks.importer.SomeError\'>: it\'s not the same object as pulpcore.app.tasks.importer.SomeError",)''.

However with this diff:

[bmbouter@localhost pulp]$ git diff platform/pulpcore/app/tasks/importer.py
diff --git a/platform/pulpcore/app/tasks/importer.py b/platform/pulpcore/app/tasks/importer.py
index 0ef18a8..023d0c3 100644
--- a/platform/pulpcore/app/tasks/importer.py
+++ b/platform/pulpcore/app/tasks/importer.py
@@ -51,6 +51,12 @@ def delete(repo_name, importer_name):
     models.Importer.objects.filter(name=importer_name, repository__name=repo_name).delete()


+class SomeError(Exception):
+    def __init__(self):
+        self.url = 'this does not matter'
+        super().__init__()
+
+
 @shared_task(base=UserFacingTask)
 def sync(repo_name, importer_name):
     """
@@ -72,4 +78,5 @@ def sync(repo_name, importer_name):

     with storage.working_dir_context() as working_dir:
         importer.working_dir = working_dir
+        raise SomeError()
         importer.sync()

When you call the sync() task you'll receive this traceback:

Aug 22 17:26:23 pulp3 celery[23021]: [2017-08-22 17:26:23,720: ERROR/ForkPoolWorker-1] Task failed : [7b33c9a3-d13f-45cf-a510-c16f3b18deeb]
Aug 22 17:26:23 pulp3 celery[23021]: [2017-08-22 17:26:23,724: ERROR/ForkPoolWorker-1] Task pulpcore.app.tasks.importer.sync[7b33c9a3-d13f-45cf-a510-c16f3b18deeb] raised unexpected: SomeError()
Aug 22 17:26:23 pulp3 celery[23021]: Traceback (most recent call last):
Aug 22 17:26:23 pulp3 celery[23021]:   File "/home/vagrant/.virtualenvs/pulp/lib/python3.5/site-packages/celery/app/trace.py", line 374, in trace_task
Aug 22 17:26:23 pulp3 celery[23021]:     R = retval = fun(*args, **kwargs)
Aug 22 17:26:23 pulp3 celery[23021]:   File "/home/vagrant/devel/pulp/platform/pulpcore/tasking/tasks.py", line 272, in __call__
Aug 22 17:26:23 pulp3 celery[23021]:     return super(UserFacingTask, self).__call__(*args, **kwargs)
Aug 22 17:26:23 pulp3 celery[23021]:   File "/home/vagrant/.virtualenvs/pulp/lib/python3.5/site-packages/celery/app/trace.py", line 629, in __protected_call__
Aug 22 17:26:23 pulp3 celery[23021]:     return self.run(*args, **kwargs)
Aug 22 17:26:23 pulp3 celery[23021]:   File "/home/vagrant/devel/pulp/platform/pulpcore/app/tasks/importer.py", line 81, in sync
Aug 22 17:26:23 pulp3 celery[23021]:     raise SomeError()
Aug 22 17:26:23 pulp3 celery[23021]: pulpcore.app.tasks.importer.SomeError
Actions #1

Updated by bmbouter over 7 years ago

I think this is an upstream Celery bug. I commented on a bug that looks very similar here: https://github.com/celery/celery/issues/3758#issuecomment-324091432

Actions #2

Updated by ttereshc over 7 years ago

This issue can't be triaged without more info/discussion.
Do we want to workaround this issue in Pulp or we are going to wait for Celery upstream fix?

Actions #3

Updated by bmbouter over 7 years ago

I want to outline two next steps that should be done for this ticket.

1) I think we need to reproduce this outside of Pulp and file upstream. Then we can let upstream identify it as a duplicate or not. I don't think we should lead the effort to resolve this in upstream Celery.

2) We should workaround the issue in the meantime while we track the status of the upstream resolution. There are two workarounds I can think of. I prefer option A.

option a) Use a factory to generate the exceptions. This is how we did it in Pulp2. It's an uncommon pattern but I think it will workaround the issue because the exception class definitions themselves won't have custom init signatures. We should not do this long-term.

option b) We can continue to define concrete exceptions but add all the necessary info as attributes by whoever instatiates the exception. This is effectively option A only with the data binding happening all over the codebase instead of in one place. I don't recommend this, but I include it for completeness.

NOTE1: All of ^ is built on the assumption that we want concrete exception classes. I think we do. If others think we don't please post here.

NOTE2: I believe switching the exception style will be backwards incompatible for the plugin API since the way they would instantiate or subclass one of the exceptions provided by the plugin API would change. With the plugin API not at 1.0 yet we can make these changes still, post release. Just an FYI related to this.

Actions #4

Updated by ttereshc over 7 years ago

  • Triaged changed from No to Yes
Actions #5

Updated by mhrivnak over 7 years ago

  • Sprint/Milestone set to 45
Actions #6

Updated by bmbouter about 7 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to bmbouter
Actions #7

Updated by jortel@redhat.com about 7 years ago

  • Sprint/Milestone changed from 45 to 46
Actions #8

Updated by mhrivnak about 7 years ago

  • Sprint/Milestone changed from 46 to 47
Actions #9

Updated by rchan about 7 years ago

  • Sprint/Milestone changed from 47 to 48
Actions #10

Updated by daviddavis about 7 years ago

  • Tags Pulp 3 MVP added
Actions #11

Updated by rchan about 7 years ago

  • Sprint/Milestone changed from 48 to 52
Actions #12

Updated by rchan almost 7 years ago

  • Sprint/Milestone changed from 52 to 53
Actions #13

Updated by jortel@redhat.com almost 7 years ago

  • Sprint/Milestone changed from 53 to 54
Actions #14

Updated by rchan almost 7 years ago

  • Sprint/Milestone changed from 54 to 56
Actions #15

Updated by bmbouter almost 7 years ago

  • Sprint set to Sprint 33
Actions #16

Updated by bmbouter almost 7 years ago

  • Sprint/Milestone deleted (56)
Actions #17

Updated by jortel@redhat.com almost 7 years ago

  • Sprint changed from Sprint 33 to Sprint 34
Actions #18

Updated by bmbouter almost 7 years ago

  • Status changed from ASSIGNED to NEW
  • Tags deleted (Pulp 3 MVP)

I am not actively working on this work so I'm marking it back to unassigned. I also believe we can fix this later since no plugin writer has raised this issue yet, so I'm removing the Pulp MVP tag.

Actions #19

Updated by bmbouter almost 7 years ago

  • Sprint deleted (Sprint 34)

Removing from sprint through email list discussion: https://www.redhat.com/archives/pulp-dev/2018-March/msg00080.html

Actions #20

Updated by bmbouter over 6 years ago

  • Status changed from NEW to CLOSED - WORKSFORME

I'm closing as NOTABUG since Pulp3 has switched to RQ. Custom exceptions should work now just fine.

Actions #21

Updated by daviddavis over 5 years ago

  • Sprint/Milestone set to 3.0.0
Actions #22

Updated by bmbouter over 5 years ago

  • Tags deleted (Pulp 3)

Also available in: Atom PDF