-
Notifications
You must be signed in to change notification settings - Fork 114
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
Integrate operations from the Notification API #472
base: main
Are you sure you want to change the base?
Integrate operations from the Notification API #472
Conversation
359804e
to
8bbd6e6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you try to rebase? Maybe this solves the test problems.
Done. |
Tests fail. Guess you need to add Line 69 in fe48906
|
Green! 🍏 |
8b3880d
to
2199b52
Compare
316c21c
to
a1f38e5
Compare
03358e4
to
aa11330
Compare
…_status operation
…fications for testing
…ferences operation
aa11330
to
7a83ae7
Compare
The root of the problem seems to be that from 24.1 notifications are handled by celery and a POST request to /api/notifications returns AsyncTaskResultSummary (or NotificationCreatedResponse) To make these tests pass I could disable celery (is it possible to disable certain tasks only?) .. but this breaks many other tests. |
Instead of disabling Celery, I suggest using the async flow. When sending a notification, we just need to poll on |
This reverts commit 7d03c6f.
a915e5d
to
bdce6a8
Compare
bdce6a8
to
31ed6ba
Compare
I changed the tests accordingly (therefor I added the tasks API). I tested manually that the tests also work for a non-celery setup. |
f9e70ac
to
a1e9cfd
Compare
This is a part of #471
Summary
How to test the changes
Run the corresponding test file:
bioblend/_tests/TestGalaxyNotifications.py