add TaskDefinition Info for global service task handler #57
Conversation
Codecov Report
@@ Coverage Diff @@
## main #57 +/- ##
==========================================
+ Coverage 73.77% 73.95% +0.18%
==========================================
Files 19 19
Lines 976 983 +7
==========================================
+ Hits 720 727 +7
Misses 236 236
Partials 20 20
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Hi @averyyan What about "task type", see https://docs.camunda.io/docs/components/modeler/bpmn/service-tasks/ Would that task type work for your use case as well? PS: when you consider refactoring your PR towards task type support - that would be awesome - |
I just have this idea from https://docs.camunda.io/docs/components/modeler/bpmn/service-tasks/. |
| @@ -286,6 +287,11 @@ func checkOnlyOneAssociationOrPanic(event *BPMN20.BaseElement) { | |||
| } | |||
| } | |||
|
|
|||
| // SetServiceTaskHandler set global service task handler to use task definition info | |||
| func (state *BpmnEngineState) SetServiceTaskHandler(handler func(job ActivatedJob)) { | |||
There was a problem hiding this comment.
This func is not mentioned in the interface.
For completeness, it should be in the BpmnEngine interface
| @@ -81,6 +82,11 @@ type TUserTask struct { | |||
| Output []TIoMapping `xml:"extensionElements>ioMapping>output"` | |||
| } | |||
|
|
|||
| type TTaskDefinition struct { | |||
| TypeName string `xml:"type,attr"` | |||
| Retries string `xml:"retries,attr"` | |||
There was a problem hiding this comment.
Since 'retries' is not used yet, I would prefer to not include it yet.
| then.AssertThat(t, definition.Retries, is.EqualTo("testValue")) | ||
| } | ||
|
|
||
| func TestServiceTaskHandler(t *testing.T) { |
There was a problem hiding this comment.
that test is not easy to understand and lacks expressiveness, how service tasks are supposed to work.
I do suggest creating a dedicated .bpmn file to showcase service tasks
There was a problem hiding this comment.
Hi,
after carefully rethinking the overall approach of a single service handler, I would like to reject this PR. Multiple thoughts made me conclude this.
First, I was inspired by your idea. Also, I fully anticipate the use case you're describing.
In general, I prefer some degree of compatibility with Zeebe. While having a single global handler, which might fit your use case, others might want to use e.g. 3 handlers for 10 tasks. This is/was the reason, why Zeebe uses taskDefintion.Type to give flexibility in the assignments.
So, I thought a lot and had so many ideas for an alternative implementation, that I gave it a try on my own. For formal reasons, I also created a new issue #58 .
I would be glad if you could have a look at the implementation and check if it fits your use case, please. (I plan to push my code changes today)
|
Not merged, in favor of 5303ef3 |
i am try to use TaskDefinition info for automatic match grpc job service.
I want to replace AddTaskHandler to job service. I think id map is not enough.