Re: [jakartabatch-dev] Please help review PR 181 for option 2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]

Re: [jakartabatch-dev] Please help review PR 181 for option 2 - status quo - Batch + CDI integration

From: "Scott Kurz" <skurz@xxxxxxxxxx>
Date: Sat, 27 Nov 2021 20:10:27 -0400
Delivered-to: jakartabatch-dev@xxxxxxxxxxx
List-archive: <https://www.eclipse.org/mailman/private/jakartabatch-dev/>
List-help: <mailto:jakartabatch-dev-request@eclipse.org?subject=help>
List-subscribe: <https://www.eclipse.org/mailman/listinfo/jakartabatch-dev>, <mailto:jakartabatch-dev-request@eclipse.org?subject=subscribe>
List-unsubscribe: <https://www.eclipse.org/mailman/options/jakartabatch-dev>, <mailto:jakartabatch-dev-request@eclipse.org?subject=unsubscribe>

> Should also mention it is only during job init - instances lookup - and not at runtime so a lazy programmatic value lookup from the bean manager/CDI.current instance will not work.

I'm not sure what you mean. What use case are you talking about here?

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

"Romain Manni-Bucau" ---11/27/2021 03:30:25 AM---Ok so value "resolution", not injection ;). +1. Should also mention it is only during job init - ins

From: "Romain Manni-Bucau" <rmannibucau@xxxxxxxxx>
To: "jakartabatch developer discussions" <jakartabatch-dev@xxxxxxxxxxx>
Date: 11/27/2021 03:30 AM
Subject: [EXTERNAL] Re: [jakartabatch-dev] Please help review PR 181 for option 2 - status quo - Batch + CDI integration
Sent by: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>

Ok so value "resolution", not injection ;). +1.

Should also mention it is only during job init - instances lookup - and not at runtime so a lazy programmatic value lookup from the bean manager/CDI.current instance will not work.

Le sam. 27 nov. 2021 à 00:10, Scott Kurz <skurz@xxxxxxxxxx> a écrit :

Romain,

I'm not sure we're talking about the same thing.

I'm trying to say in the spec that for this batchlet/bean:

//@ApplicationScoped
@Dependent
public class MyBatchlet extends AbstractBatchlet {

@Inject @BatchProperty(name = "a")
String myProp;

and this JSL:

the 'myProp' value injected will be based on job parameter 'paramA', thus not known until execution time.
More generally, the @BatchProperty Beans will follow the JSL substitution rules for a Dependent-scoped artifact.

In contrast, if the batch artifact Bean is defined with something other than Dependent-scoped, then the property values will be undefined.
I like that we're clarifying a certain behavior around JobScoped/StepScoped, even if we've run out of time to add them to 2.1.

I'm not really trying to say much about an override case. If the user wants to do something different I'm not trying to stop them in the spec.

But I'm saying the batch runtime should provide a Bean implementing the above behavior.

-----------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience

skurz@xxxxxxxxxx

--------------------------------------------------------

"Romain Manni-Bucau" ---11/26/2021 04:54:44 PM---Please never interprete @Inject without cdi in cdi beans, it only break cdi and you will end up prev

From:

"Romain Manni-Bucau" <

rmannibucau@xxxxxxxxx

To:

"jakartabatch developer discussions" <

jakartabatch-dev@xxxxxxxxxxx

Date:

11/26/2021 04:54 PM

Subject:

[EXTERNAL] Re: [jakartabatch-dev] Please help review PR 181 for option 2 - status quo - Batch + CDI integration

Sent by:

"jakartabatch-dev" <

jakartabatch-dev-bounces@xxxxxxxxxxx

Please never interprete @Inject without cdi in cdi beans, it only break cdi and you will end up preventing users to actually use cdi. Keep in mind cdi enables to override all beans at startup, including jbatch ones, this would be broken or require extreme caution I understand you want to bypass.

Not aligning cdi and standalone case is fine since standalone is a degraded case. An example is that constructor injection is not supported and nobody really cares so let's stay simple and integrated, no?

Le ven. 26 nov. 2021 à 21:58, Scott Kurz <

skurz@xxxxxxxxxx

> a écrit :

Thanks Romain and Roberto so far for the reviews.

Based on that I'd feel pretty comfortable merging this, except, ...

Let me highlight one point I want to make sure we agree on, based on Romain's comment:

https://github.com/eclipse-ee4j/batch-api/pull/181#discussion_r756388801

When a batch artifact is loaded as a CDI bean implying each @BatchProperty is injected with a Bean, we've discussed how for non-Dependent-scoped beans the batch property values are going to be unpredictable. E.g. an ApplicationScoped bean's properties could be injected well before the job begins executing.

I captured that here:
h

ttps://github.com/scottkurz/batch-api/blob/add-cdi-req/spec/src/main/asciidoc/batch_programming_model.adoc#properties-for-non-dependent-scoped-bean-instances-undefined

But for a Dependent-scoped batch artifact loaded as Bean, I think each @BatchProperty value should be set exactly as it is in the non-bean case. Since there's no need to instantiate this bean until job execution (and more precisely the execution of this artifact within the job), I think these values should reflect all the JSL substitution capabilities, incorporate job parameter values, etc.

I wrote that up here, pointing to the non-Bean section:

https://github.com/scottkurz/batch-api/blob/add-cdi-req/spec/src/main/asciidoc/batch_programming_model.adoc#properties-for-dependent-scoped-bean-instances

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

skurz@xxxxxxxxxx

--------------------------------------------------------

"Scott Kurz" ---11/24/2021 11:43:22 AM---OK, so first, apologies if you've only been watching this Batch ML as a lot of recent discussion on

From:

"Scott Kurz" <

skurz@xxxxxxxxxx

To:

jakartabatch-dev@xxxxxxxxxxx

Date:

11/24/2021 11:43 AM

Subject:

[EXTERNAL] [jakartabatch-dev] Please help review PR 181 for option 2 - status quo - Batch + CDI integration

Sent by:

"jakartabatch-dev" <

jakartabatch-dev-bounces@xxxxxxxxxxx

OK, so first, apologies if you've only been watching this Batch ML as a lot of recent discussion on Batch+CDI integration in the last couple weeks had moved to the Platform ML, as we sought Platform input.

In spite of the poll still being open, given the time constraints and the late date, I wanted to go ahead and draft a spec update PR for the "Option 2" status quo path.

This is at:

https://github.com/eclipse-ee4j/batch-api/pull/181/

IMHO, I think it's easier to review by building the new spec document and reading it straight through than looking at the diff/compare.
`mvn install -Dmaven.javadoc.skip=true` # output in spec/target/generated-docs

I left some review notes highlight the changed sections and key questions in the GitHub comment.

----

BTW, if anyone is looking to help at this point, I think adding TCK tests covering the newly-specified items would be a helpful way to contribute.
Though this may be the impl status quo.. it is of course not covered by the TCK (well, maybe there's a few assumptions implicitly tested) and a decent amount of new material.

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

[attachment "graycol.gif" deleted by Scott Kurz/Poughkeepsie/IBM] [attachment "graycol.gif" deleted by Scott Kurz/Poughkeepsie/IBM]

_______________________________________________ jakartabatch-dev mailing list