- Issue created by @mxh
- last update
over 1 year ago 297 pass - @mxh opened merge request.
- Status changed to Needs review
over 1 year ago 9:18pm 16 August 2023 - 🇩🇪Germany mxh Offenburg
Created a first MR draft that unifies the three initialization layers mentioned in the IS, by using one state value. Within that unification refactor, I renamed some (newly introduced) methods and commands, as it seems to make more sense being named that way.
It also seems that we can get rid of the event's applies method entirely, and solely rely on the lazy event appliance check. Of course this is a major API change that requires any integration to adapt their event classes. However that will happen anyway when we plan to move such logic into the event plugin implementation instead. Probably makes sense to see this issue as a blocker for the event plugin refactoring when we want to make this change.
The
EcaStorage
class is now way simpler and does not contain any event initialization logic, this is now completely covered by the Processor, where it belongs to. Also removed the memory cache implementation for event objects from theEca
configuration entity, because it doesn't make sense to cache them when looking into the code parts that invoke /ask for ECA event objects. This reduces a bit of I/O overhead because no cache tags are involved anymore for those objects. - Issue was unassigned.
- last update
over 1 year ago 297 pass - First commit to issue fork.
- last update
over 1 year ago 297 pass - Status changed to RTBC
over 1 year ago 10:54am 18 August 2023 - 🇩🇪Germany jurgenhaas Gottmadingen
This is astonishing! I like this approach a lot, and I've not figured a single issue by reviewing this. Only a couple of code style minors, that I've fixed and committed in a single commit.
Yes, testing is required. I tend to suggest that we merge the MR and tackle potential issues in follow-up issues separately, simply because that makes testing much easier, also fort other folks hanging around. What do you think?
- 🇩🇪Germany mxh Offenburg
That's great to hear, and thanks for looking into this. I'd also vote for merging this. Then also further refactoring can be addressed, such as moving ECA-specific logic like
ConditionalApplianceInterface
and others completely into the according ECA plugin implementation. - last update
over 1 year ago 297 pass -
jurgenhaas →
committed 67f6faa4 on 2.0.x authored by
mxh →
Issue #3381551: Performance optimization: Unify the event and process...
-
jurgenhaas →
committed 67f6faa4 on 2.0.x authored by
mxh →
- Status changed to Fixed
over 1 year ago 11:36am 18 August 2023 - 🇩🇪Germany jurgenhaas Gottmadingen
OK, merged this and added the issue to the CR for later detailed explanation of what module maintainers need to update in their modules.
Automatically closed - issue fixed for 2 weeks with no activity.