Feat: Implement Temporal

[praswicaksono]

Q	A
Branch?	3.x
Bug fix?	No
New feature?	Yes

Integrate temporal worker and client. It detect if temporal/sdk was loaded it will enable integration. Here list following feature that have been done in this PR

Client

• Configure client
• Ability to connect with SSL (required to connect with temporal cloud)
• Register client interceptors
• Multiple client (ability to create multiple client for example: we can create local client and cloud client
• Configure data converter

Worker

• Configure worker
• Register worker interceptors
• Configure exception interceptor
• Configure data converter
• Multiple worker (workflow or activity can be assigned to specific worker)

[Baldinof]

Thank you for opening this.

I'll try to review it this week and make it happen (sorry I'm not much into the PHP ecosystem these days, so it may take time).

[root-aza]

@praswicaksono Hi! Why weren't you happy with this decision?
https://github.com/VantaFinance/temporal-bundle

[Nyholm]

I know you opened this PR a long while ago. But can you make it easier for us to review by:

1. Rebase the PR
2. Dont change CS
3. Dont change (or minimal only) to the CI files

[crazy-weasel]

Hi!

I did the 3 suggested steps in my own fork: https://github.com/crazy-weasel/roadrunner-bundle/tree/feat/implement-temporal
and additionally I:

• fixed a typo
• separated the creation of the ServiceClient into its own Factory, so that I can also create a ScheduleClientFactory

Before I create another pull request I want to be sure if that's okay, or if can be merged with this pr here. I'm not very active on GH and have no real experience with PRs. :)

Also, there are some topics that I am not so sure about:

1. the services from this PR have been marked as public. I did not change that (yet), but I think it is sufficient if the services are private. (see)
2. so far this feature only registers Workflows when the WorkflowInterface-Attribute is applied to a class. Using the recommended structure of splitting the workflows into interface and implementation will not work. I'm not sure how that can be changed, and as I'm only learning on how to use Temporal I'm not sure if it is necessary too.
3. There are no tests for this feature yet.
4. I did not want to change too much of the original PR, and therefore I added the services for the ServiceClientFactory and ScheduleClientFactory etc. to the already existing naming scheme for the services. (e.g. "temporal.client.$name.schedule")

Thank your for any input, guidance and help on this.

[crazy-weasel]

On further reading the code, I found nothing resetting the kernel or checking on doctrine. I guess that could added via the registerActivityFinalizer Method on the worker from the temporal SDK.

Tomorrow I'll digg more into the code of the full bundle.

[crazy-weasel]

Instead of using the activityFinalizer I created two ActivityInboundInterceptors, similar to the middlewares for the HttpWorker.

One that is similar to the finally-block of the start-Method of HttpWorker, so that the reboot-strategies can also be used in the TemporalWorker.

And one that mirrors the already existing DoctrineORMMiddleware. For that one I extracted the preRequest and postRequest-Methods in a trait to avoid duplicate code.

The code for configuring those interceptors is already present in its current state in the PR, however, similar to the middleware for the HttpWorker, I believe this functionality should be built-in and not require implementation by the users of the bundle.

I encountered an issue with an closed EntityManager. Only a call to managerRegistry->resetManager made it possible to use the EntityManager in further activities. I’m unsure how this is handled in HttpWorker, aside from what’s implemented in the middleware mentioned above.

Feedback, some clarification or hints on where I could inform myself is very much welcome. :) Also, I’d love to know if you think my modifications fit well with the bundle.

I plan to push my modifications to my fork tomorrow. I still need to clean up some changes I made to the configuration, but I wanted to share my findings so far.

[crazy-weasel]

Hi again,

I just pushed my changes to my fork - I can create a new pull-request for that, or is it possible to "merge" it with this one? I don't want to take away something away from praswicaksono.

I solved the previously mentioned problems by registering the activities with a factory function that get's them form the container. Previously, the activities were stuck with old dependencies after a kernel-reboot, because the activities themselfs were built with the old container.

There was no reliable way to update the dependencies of the TemporalWorker after a reboot, that's why I'll always get the dependencies directly from the container. Like with the other workers, I created a class that collects all dependencies for the TemporalWorker. The activities are provided by a service locator.

I tried updating them by listening to the KernelRebooted-Event but for some reason that was not reliable. I think the inner workings of the worker from the temporal sdk are the reason for that.

Looking forward to your feedback and instructions on what's the best way to integrate it with the already existing work.

[Nyholm]

If you push your new changes to praswicaksono:feat/implement-temporal, then this merge request will be updated.

[crazy-weasel]

looks like I can't - I get an error 403 (afterall it's not my branch?)

do you think there is a way around this? If there is a better way, I'd just repeat the work that I did (the rebase, and the modifications to the code)

Or should I make a new pull-request? I checked, praswicaksono is still listed as an author in my branch. :)

Sorry, if I made this to messy!

[Nyholm]

Oh, sorry. I misread, You are not the author of that branch of course. Just open a new PR and you will be fine =)