-
Notifications
You must be signed in to change notification settings - Fork 125
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
Decouple serialisation-related logic from AsyncHandler #1334
Comments
Potentially related to #945 |
I can totally feel your pain with regard to current async handlers implementation and various limitations coming from the framework it leans on: What works for me, which can be considered as a hack, is to: class MyEventProcessingJob < ApplicationJob
prepend RailsEventStore::AsyncHandler
def perform(event)
call(event)
end
def call(event)
# ...
end
end or — to see it clearly — inlined: class EventHandler < ApplicationJob
def perform(payload)
event = event_store.read.event(payload.symbolize_keys.fetch(:event_id))
call(event)
end
end
class MyEventProcessingJob < EventHandler
def call(event)
# ...
end
end and then:
I consider this a hack because of instantiating framework class (inheriting from ActiveJob after all) and its a bit of grey area. Like every hack, what is acceptable for me might not be acceptable by someone else. |
Just to expand a bit:
|
Also to keep in mind, a likely direction we'll take in the future: #755 (comment) |
Perhaps there's also some quick win with globalid and its support in ActiveJob: https://edgeguides.rubyonrails.org/active_job_basics.html#globalid |
That said, I'm aware that async handler experience can be improved here. I'm not convinced yet with proposed implementation. I'd also be very cautious when introducing changes here — we'll have to support them for a longer term in the future. I'll keep the issue open as a reminder. |
I don't know enough about the internals of this gem yet (and I'm happy to delete my comment if this is not helpful context), but I ran into an error I think is related to this issue. It seems like async handlers can't handle OpenStruct object types as we end up with this error: NoMethodError (undefined method `fetch' for "OpenStruct":String):
ruby_event_store (2.9.1) lib/ruby_event_store/mappers/transformation/preserve_types.rb:147:in `block in restore_types' I can work around it of course, but it would be great if I was able to poke that through the un-modified OpenStruct somehow. An external API I'm consuming is returning OpenStructs and I am hoping to keep the integration layer there as slim as possible. |
@msencenb Can you please share some more bits of the code — i.e. RailsEvenStore instance configuration and the sample event that is causing this? I'm happy to help, executable code to reproduce the issue you're experiencing would make it much easier. |
@msencenb I've managed to recreate the issue you're experiencing: https://gist.github.com/pawelpacana/f39026514185e8f72e4daaca2d99b85a I'll look into it more in the upcoming days. It seems that I'm still guessing what is your exact configuration (RailsEventStore, database engine, etc.). |
@msencenb There's a workaround which might work without introducing too much changes:
That is using YAML only for particular type for storage. It works because it fits into current shape assumptions of
|
@pawelpacana Thank you and apologies for the slow response. You reproduced what I have, here is my config, which is very minimal and using the JSONClient. require "rails_event_store"
Rails.configuration.to_prepare do
Rails.configuration.event_store = RailsEventStore::JSONClient.new
# Subscribe event handlers below
Rails.configuration.event_store.tap do |store|
store.subscribe(WiqListeners::Justifi::SubAccountEventHandler, to: [WiqEvents::Justifi::SubAccountUpdated])
store.subscribe_to_all_events(RailsEventStore::LinkByEventType.new)
store.subscribe_to_all_events(RailsEventStore::LinkByCorrelationId.new)
store.subscribe_to_all_events(RailsEventStore::LinkByCausationId.new)
end
end I'm using Postgres as my data store. In my case I was able to just poke through an id in my event and re-pull the external resource from my partner's API. |
Restoring type shouldn't rely on stored argument (data). It should solely rely on type that was persisted in metadata. E.g. using OpenStruct for data failed before this change, see: #1334 (comment) As a sideeffect, two obsolete conditionals could be removed. Co-authored-by: Łukasz Reszke <[email protected]> Co-authored-by: Paweł Pacana <[email protected]>
wow that was fast—thank you for the patch, I appreciate it 😃 |
Trying to use the async handler in my ActiveJob class like so:
This works flawlessly in a regular setup: the
perform
method receives instances of the ActiveRecord-based event model.But now it's not possible to use the job manually anymore: calling
MyEventProcessingJob.perform_now(some_event)
will raiseundefined method
symbolize_keys'` because that's not a method of the Rails model class.The current implementation reads
I propose moving the
**
andsymbolize_keys
somewhere else, probably into the serializer.That way, when using the
NULL
serializer and calling perform with an event instance, the same event instance will end up in the job class.The text was updated successfully, but these errors were encountered: