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

Implement ActiveJob adapter #17

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

vovimayhem
Copy link
Contributor

Implements the ActiveJob adapter for Subserver to process ActiveJob jobs

@vovimayhem
Copy link
Contributor Author

Hi @hagmic, @ wintheday! I'm interested on any comments & feedback about this!

@paradigm314
Copy link
Contributor

@vovimayhem This is awesome, I would love to add ActiveJob support to Subserver. I will take a look at this and let you know.

@paradigm314 paradigm314 added the needs review Needs code review from a maintainer label Jan 15, 2020
@vovimayhem
Copy link
Contributor Author

(Keep in mind this is still a "draft")

@paradigm314
Copy link
Contributor

@vovimayhem I looked over this and everything is looking good. No Feedback at the moment, I am curious how you plan on wrapping Jobs in with the JobWrapper. Looking forward to seeing more when you have the time.

@vovimayhem
Copy link
Contributor Author

@WintheDay I'm currently running jobs OK using this branch, but I haven't checked all the features I think are expected from any ActiveJob adapter - for example, I haven't figured out yet how specifying a queue might work with subserver.

The JobWrapper class is currently very simple. Job data is fed by ActiveJob to the wrapper, and the wrapper executes the given job.

vovimayhem added a commit to IcaliaLabs/icalia-sdk-ruby that referenced this pull request Mar 8, 2020
* Removed Icalia::Event::Notification

* Renamed the subscriber auto subscribe method, due to changes in the Subserver PR

See:
- lifechurch/subserver#17
- lifechurch/subserver@8d2ebbe
- lifechurch/subserver@a59caef
@vovimayhem
Copy link
Contributor Author

vovimayhem commented Mar 13, 2020

I think I'll be extracting the changes required to make "auto-subscribing subscribers" in another pull request, in order to concentrate on testing of those first, and release changes in small-but-valuable packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Needs code review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants