Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add generic support: item/checkpoint type for reader/processor/writer/listener classes, plus user data types #13

Open
scottkurz opened this issue Mar 7, 2020 · 10 comments · May be fixed by #18

Comments

@scottkurz
Copy link
Contributor

Brought up discussion here to understand what the EE 9 direction is.

Maybe it would be better to include this change along with the package rename.

I'll add more detail if/when we decide this is a valid candidate.

@chengfang
Copy link
Contributor

I think it's a good candidate for improvement. Now seems a good time to do it since we are renaming packages and making breaking changes. It will be worthwhile to prototype the changes in jbatch and see what they entail.

Will the batch artifacts with generics be specified differently in batch.xml, and job.xml where a fully-qualified class name is needed?

Should the spec say anything to enfore item reader, item processor, and item writer all have the consistent generic type, or just let it fail with the default exception from java?

@rmannibucau
Copy link

I'd add another question - and obviously I am +1 for this issue: should we enable to register implicit converters on the job or globally (app/cdi context) to enable the reuse of jbatch components?

@scottkurz scottkurz changed the title Add generic support to ItemReader/Writer/Processor, and for user data types. Consider adding generic support for the 1.0 - equivalent API Mar 10, 2020
@scottkurz
Copy link
Contributor Author

I'm worried we might be a bit late to this decision.

Though we'd need to construct our own release plan, let's say we were willing to take on the additional process overhead.

Do we know exactly what we'd want the API to look like?

I dug through the old JSR 352 git repo and can point to this compare which shows the API before we removed generics, which I'd imagine closely mirrors Spring Batch.
https://github.com/scottkurz/jsr352-java-net-backup/compare/after-generics-removed..scottkurz:before-generics-removed?w=1
(Sorry there are some unrelated changes in that diff)

Is this what we'd want ?

That's all I can offer today, will look more soon.

Note though that other specs have decided to punt on this until post-EE9:
E.g. https://www.eclipse.org/lists/jakartaee-platform-dev/msg01608.html
though obviously there's something to be said for making this other breaking change along with the package rename.

Also, since no other Jakarta API depends on Batch, we have that working in favor of having more freedom here.

@scottkurz
Copy link
Contributor Author

@rmannibucau

should we enable to register implicit converters on the job or globally (app/cdi context) to enable the reuse of jbatch components?

could you please give an example of what you mean? If it's not closely related to the generics idea I'd just say we should defer to post-EE9 but I'm not sure I really am following. Thx.

@rmannibucau
Copy link

@scottkurz sure, if you take a simple "chain" reader R -> writer W. Today both use objects so they can coerce as they want and if there is a class cast exception it is user code, not jbatch. Tomorrow R will issue a Record and W will read a Map (for example) because I reuse R and W from a batch component library. Therefore runtime (jbatch impl) will fail.
Indeed we can say user can wrap the components but it is against the reusability and the whole component model (if so we would better inject a jbatch state saver than handle the flow in the framework).
This is where the ability to register implicit mappers/converters when launching the job+in cdi comes from.
Side note: can also require to define a generic Record type to have implicit coercing as Apache Beam/Spark/Flink and most workflow frameworks do to be able to work on generic flows but it can comes later since we wouldnt break composition with converters at least.

Hope it makes sense.

@scottkurz
Copy link
Contributor Author

scottkurz commented Mar 10, 2020

I agree this seems like a good focus area where there's opportunity for improvements. I think we want to look at when to incorporate existing CDI constructs like maybe interceptors/decorators and when to invent things that are more batch specific.

I'd rather decouple this from the generics discussion, though. It seems like it could stlil be an incremental step forward to define generic-typed artifacts, and if later we added more item conversion capabilities, that would be even better. And mainly, it's because I don't see us resolving this in time. The 2.0 release date on our https://projects.eclipse.org/projects/ee4j.batch project page is 2020-04-01, so I'm not even sure the generic change is doable.

Opened #16 to follow-up on the item conversion ideas.

@rmannibucau
Copy link

My understanding is we can do a 2.0 iso the previous one and then do 2.1, 2.2 etc independently of jakarta and jakarta will catch up the version after so maybe just push all new features for after?

@scottkurz scottkurz changed the title Consider adding generic support for the 1.0 - equivalent API Add generic support: item/checkpoint type for reader/processor/writer/listener classes, plus user data types Mar 11, 2020
@scottkurz
Copy link
Contributor Author

We should look at ideas that would not require breaking source compatibility, such as doing this in a set of subclasses as in:
https://github.com/apache/geronimo-batchee/blob/master/extensions/extras/src/main/java/org/apache/batchee/extras/typed/TypedItemReader.java#L28

Maybe there's a way to leverage interface default methods too.

@scottkurz
Copy link
Contributor Author

Linked the text from the original JSR 352 issue, in a re-opened issue: #83 (which I closed as dup).

@scottkurz scottkurz linked a pull request Oct 5, 2021 that will close this issue
@scottkurz
Copy link
Contributor Author

While deprecating the existing, non-generic-typed interfaces, as in PR #18, is one option, I think we should make sure we have a bit more review and maybe discussion of this aspect. Thank you for adding the PR though Romain.

@scottkurz scottkurz removed this from the Batch 2.1 (proposed) - EE 10 milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants