Re: [jakartabatch-dev] discussion on JobOperator injection - should runt

["Scott Kurz" <skurz@xxxxxxxxxx>]

Since we don't have anything more than a single classpath, and single "app start" in the TCK today.. this is going to be a non-trivial TCK update.. even though I see how the spec update could be done.

I'm going to ignore that for now, and work to get a TCK release out the door this week.
  I opened https://github.com/eclipse-ee4j/batch-api/issues/190 to follow-up for the final 2.1 release.

Thanks Romain especially for your input here !

------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------

"Scott Kurz" ---12/15/2021 01:04:05 PM---OK, I see now what you are thinking. This seems feasible and I do like the idea of not breaking exis

From: "Scott Kurz" <skurz@xxxxxxxxxx>
To: "jakartabatch developer discussions" <jakartabatch-dev@xxxxxxxxxxx>
Cc: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>
Date: 12/15/2021 01:04 PM
Subject: [EXTERNAL] Re: [jakartabatch-dev] discussion on JobOperator injection - should runtime provide bean? , extensions, portability, etc.
Sent by: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>

---

OK, I see now what you are thinking.

This seems feasible and I do like the idea of not breaking existing apps that have worked around the previous limitation.   

The spec would need to make a stronger statement than how I have it worded now.

I'd like to get another opinion or two from the general CDI and/or platform perspective before we wrap up the final release.

But I like this working assumption. 

Now I have to tackle the TCK....

------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------

"Romain Manni-Bucau" ---12/15/2021 11:11:23 AM---Le mer. 15 déc. 2021 à 16:49, Scott Kurz <skurz@xxxxxxxxxx> a écrit : > I understand the backward co

From: "Romain Manni-Bucau" <rmannibucau@xxxxxxxxx>
To: "jakartabatch developer discussions" <jakartabatch-dev@xxxxxxxxxxx>
Cc: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>
Date: 12/15/2021 11:11 AM
Subject: [EXTERNAL] Re: [jakartabatch-dev] discussion on JobOperator injection - should runtime provide bean? , extensions, portability, etc.
Sent by: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>

---

Le mer. 15 déc. 2021 à 16:49, Scott Kurz <skurz@xxxxxxxxxx> a écrit :
I understand the backward compat intent but I don't see how to make it work.   It's possible I'm missing some CDI tools that make this feasible.

In pseudo code it looks like https://gist.github.com/rmannibucau/da243d7ebcafd294438700e311286502
 


The idea of checking in the runtime BeforeBeanDiscovery observer seems to have a problem:  
How can we guarantee that the runtime Extension observer runs AFTER the application Extension observer?     Is there a Priority we can specify that would guarantee this?
If it can't be specified decisively and portably then we're back to where I was in my suggestion:   leaving this up to the implementation/vendor.


The CDI extension lifecycle enables it, can need some more type checking like "it is the internal bean so we ignore it" but nothing crazy for an impl.
 


I don't believe it is possible to use the AfterBeanDiscovery to delete a Bean (I found this issue but don't see a new EE4J/Jakarta issue:  https://issues.redhat.com/browse/CDI-585)


Nop but you use it to register if no other bean was found after the discovery (so no new bean should be found statically).
It has a small issue for programmatic registration but all the programmatic registrations should have this guard so we are good anyway.
 


Are there other ideas?

I think we're really talking about a distinction between "polite" Extensions that defer to another vs. "aggressive" Extensions that don't... rather than a distinction between "app" vs "runtime" Extensions.    And we're saying a batch runtime should provide a "polite" Extension.


Exactly, you can also rename it "backward compatible" versus "breaking".
 


We didn't go into any of this detail in the injection of batch properties and contexts.   I guess practically they're less likely to collide but at some level it seems like why wouldn't we also have to consider this?


For that reason, it is unlikely but we can have the same reasoning if you want, it is just less common from my experience so maybe not worth it.
 

---

I do think whatever we decide needs to be enforced by the TCK.      So it might help also to think what the TCK test will be to validate this discussion.


@Produces MyJobOperator /* not JobOperator but an implementation in @Dependent scope */ op() { return new MyJobOperator(); }

and in a batch component (batchlet?): @Inject JobOperator op; -> check the impl is MyJobOperator (thanks @Dependent scope it is possible).

Another test checks job operator injection is there when the producer of MyJobOperator is not present (so the default bean is provided).
 


I've been thinking in terms of doing an @Inject JobOperator in a TCK artifact without defining a TCK-level Extension.

To verify Romain's idea, OTOH, I think we would need to do the above test AND ALSO define a TCK-level extension with its own JobOperator producer, and validate the fact that its JobOperator instance gets injection rather than any supplied by the runtime.

(And of course we need a way to isolate the two cases.. since today we basically have one big classpath.       Not trying to argue against this because it'd be more work for the TCK to implement.   Just observing).


Right but since the TCK define packaging it sounds very doable, no?
 
------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------

"Romain Manni-Bucau" ---12/15/2021 09:36:08 AM---Le mer. 15 déc. 2021 à 15:00, Scott Kurz <skurz@xxxxxxxxxx> a écrit : > What does "user did the work

From: "Romain Manni-Bucau" <rmannibucau@xxxxxxxxx>
To: "jakartabatch developer discussions" <jakartabatch-dev@xxxxxxxxxxx>
Cc: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>
Date: 12/15/2021 09:36 AM
Subject: [EXTERNAL] Re: [jakartabatch-dev] discussion on JobOperator injection - should runtime provide bean? , extensions, portability, etc.
Sent by: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>

---

Le mer. 15 déc. 2021 à 15:00, Scott Kurz <skurz@xxxxxxxxxx> a écrit :
What does "user did the work" mean though?    Are we suggesting an ordering based on classloader ?   We've not gotten into any classloader hierarchies in Batch.. I'm not really familiar with what's already specified generally in the Platform (though familiar of course with particular implementations).


No, nothing which can not be portable accross EE, just the fact the user created a CDI bean which matches @Default JobOperator (like previous producer).
 


Given that we have CDI couldn't we leave it up to the user to give priority/veto, and/or set the right alternative if there is more than one?    This would also open the door for a completely impl/vendor-specific mechanism to disable the runtime-provided bean as well.


If I rephrase it means: do you want the upgrade to be transparent or do you want the user to have to reconfigure/code the app to do it?
EE rules are more on backward compat side in general - even if eclipse kind of breaks everything I think it is good to try to stick to it.
 


------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------

"Romain Manni-Bucau" ---12/15/2021 08:50:20 AM---Hi Scott, the common pattern is:

From: "Romain Manni-Bucau" <rmannibucau@xxxxxxxxx>
To: "jakartabatch developer discussions" <jakartabatch-dev@xxxxxxxxxxx>
Cc: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>
Date: 12/15/2021 08:50 AM
Subject: [EXTERNAL] Re: [jakartabatch-dev] discussion on JobOperator injection - should runtime provide bean? , extensions, portability, etc.
Sent by: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>

---

Hi Scott,

the common pattern is:

@Produces JobOperator op() { return BatchRuntime.getJobOperator(); }

Sometimes with @ApplicationScoped added too.

This is why I think not adding the bean is the simplest and most user friendly solution for end users (avoids a @JBatch qualifier for ex).

In terms of priority it is quite simple: if user did the work it is always higher priority than the default runtime one. If you need more - which is out of spec cause it means multiple beans are in user land - you have CDI to solve it so we are good, simple and safe with it I guess?

Romain Manni-Bucau
@rmannibucau |  Blog | Old Blog | Github | LinkedIn | Book


Le mer. 15 déc. 2021 à 14:44, Scott Kurz <skurz@xxxxxxxxxx> a écrit :
Roberto, 

Thanks for your review.   However, I'm not clear which side you're taking...  which probably means I haven't laid out the decision clearly enough.

Romain had suggested https://github.com/eclipse-ee4j/batch-api/pull/183#pullrequestreview-828924368  
specifying the runtime should ONLY register a JobOperator bean if none is present.

I was trying to be a bit more vague, and say the application should expect the runtime to provide one, but wanted to avoid saying anything about the precedence. 
I don't think the precedence is easy to specify.   How does a given piece of code know whether it is the "runtime" that should defer to the "application" JobOperator or vice versa?

But from your response, I wonder if you're arguing for Romain's suggestion. 

When you say you always include add a JobOperator bean.. do you just have it delegate to BatchRuntime.getJobOperator() ?   Or do you delegate to your specific JobOperator impl (in which case it doesn't sound portable). 

Thanks,
------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------

"Roberto Cortez via jakartabatch-dev" ---12/14/2021 07:50:07 PM---I think it is reasonable for implementations to provide injection for JobOperator if none is present

From: "Roberto Cortez via jakartabatch-dev" <jakartabatch-dev@xxxxxxxxxxx>
To: "jakartabatch developer discussions" <jakartabatch-dev@xxxxxxxxxxx>
Cc: "Roberto Cortez" <radcortez@xxxxxxxxx>
Date: 12/14/2021 07:50 PM
Subject: [EXTERNAL] Re: [jakartabatch-dev] discussion on JobOperator injection - should runtime provide bean? , extensions, portability, etc.
Sent by: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>

---

I think it is reasonable for implementations to provide injection for JobOperator if none is present. It is likely that a lot of apps out there provide their own produces, I think I always add one every time I use batch :)
On 13 Dec 2021, at 15:24, Scott Kurz <skurz@xxxxxxxxxx> wrote:

First, a status update:  

The spec and TCK have been merged with several CDI integration updates.     Thank you especially to Romain for all the reviews and helpful comments.

I am very close to releasing a 2.1.0-M1 TCK.     At that point, as impls experiment with the TCK we can go back and see if any of the spec/TCK updates are needed, and discuss more, etc.
(I'm sure not everyone got to review every line of spec update merged so far... so there's still a chance to make changes).
---

There's only issue we need to discuss though:  injecting JobOperator.

My general thought was to establish that a runtime should provide a JobOperator while saying as little about the mechanism as possible.

This mirrors the approach we did for the BatchRuntime.getJobOperator() API, as we did not specify the extension mechanism here either.
(Though of course the javax/jakarta Maven artifacts did use ServiceLoader and so arguably provide a "de facto standard"?)

So I kept the language vague in the PR:  https://github.com/eclipse-ee4j/batch-api/pull/183 saying no more than that the runtime must provide a JobOperator CDI Bean "if necessary".

I also added a new TCK test to inject JobOperator into an executing batch artifact (I didn't want to complicate the TCK by some other servlet or managed component type to the test).

However, Romain pointed out in comments:
 https://github.com/eclipse-ee4j/batch-api/pull/183#issuecomment-991316256
 https://github.com/eclipse-ee4j/batch-api/pull/183#pullrequestreview-828924368

that this could potentially break an existing user packaging their own JobOperator and suggested we might say something like "the runtime only adds a bean if it detects there is not already one present".

IMO, we should not detail the precedence or extension mechanism here.... but instead leave this up to vendors/implementations.

OTOH, I do not think we should remove the requirement altogether.   I think the user should expect that a simple app can be deployed to a runtime that claims batch support and the runtime will have a JobOperator ready for it to inject.

OK, let me stop and hit send and see what everyone else thinks.

Thanks,
------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------

_______________________________________________
jakartabatch-dev mailing list
jakartabatch-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartabatch-dev

_______________________________________________
jakartabatch-dev mailing list
jakartabatch-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartabatch-dev




_______________________________________________
jakartabatch-dev mailing list
jakartabatch-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartabatch-dev_______________________________________________
jakartabatch-dev mailing list
jakartabatch-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartabatch-dev

_______________________________________________
jakartabatch-dev mailing list
jakartabatch-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartabatch-dev_______________________________________________
jakartabatch-dev mailing list
jakartabatch-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartabatch-dev

_______________________________________________
jakartabatch-dev mailing list
jakartabatch-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartabatch-dev_______________________________________________
jakartabatch-dev mailing list
jakartabatch-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartabatch-dev

_______________________________________________
jakartabatch-dev mailing list
jakartabatch-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartabatch-dev