[#BATCH-366] Do we still need StepInstance?

Spring Batch

Do we still need StepInstance?

Created: 15/Feb/08 03:57 AM Updated: 07/Aug/08 10:06 AM

Component/s:

Core

Affects Version/s:

1.0.0.m5

Fix Version/s:

1.0.0.m5

Time Tracking:

Original Estimate:

0.5d

Remaining Estimate:

0.5d

Time Spent:

Not Specified

Issue Links:

Depends

This issue depends on:

~~BATCH-364~~ StepScope responsibilities can be assumed by Step (not ApplicationContext)

Description

« Hide

StepInstance has lost weight over time to the point where I think it's mere existence needs to be questioned.

Currently the only information StepInstance essentially provides is the step name. The execution count and last execution can be seen as convenience values that can be retrieved by different means - classes asking the step instance for these values (SimpleJob and StepExecutor/ChunkedStep) already have reference to the repository.

Technically it seems easy to add 'stepName' property to StepExecution and live with the pair Step+StepExecution (with already existing link StepExecution -> JobExecution -> JobInstance) rather than triple Step+StepInstance+StepExecution.

Alternatively it might be StepInstance is missing properties that would logically belong to it and justify its existence, but I can't think of any candidates.

All

Comments

Work Log

Change History

FishEye

Builds

Sort Order:

[ Permlink | « Hide ]

Dave Syer added a comment - 15/Feb/08 04:22 AM

It is pretty important that ExecutionAttibutes are stored against the step instance. If the Dao can handle copying the last execution values into the new one, then at that point I agree that StepInstance is redundant. If you can do that, then I am happy to see StepInstance die.

[ Permlink | « Hide ]

Dave Syer added a comment - 15/Feb/08 06:21 AM

You might not need to persist it, but you will need a new method in the JobRepository (or something). The tricky bit is this: on restart the Step has to restore from the *previous* step execution (not the current one, clearly), so there needs to be a path from the step execution to the previous one if it exists. I think this might be broken at the moment anyway, but if we remove StepInstance, this has to be addressed. Please hold off doing that until I have finished with ~~BATCH-364~~.

[ Permlink | « Hide ]

Lucas Ward added a comment - 15/Feb/08 10:03 AM

Dave,

ExecutionAttributes aren't currently tied to the StepInstance, rather, it's tied to the last execution of a step instance, which *could* be a job instance, assuming you could realiably get the last execution back (which I believe you can).

[ Permlink | « Hide ]

Dave Syer added a comment - 18/Feb/08 01:33 AM

So I would get the last execution via stepExecution.getJobExecution().getJobInstance().getLastStepExecution()? If that works it can be shortened (with some null checks) to stepExecution.getLastExecution(). I am quite happy with that - can you get a comment from Lucas and/or Ben?

[ Permlink | « Hide ]

Robert Kasanicky added a comment - 18/Feb/08 02:36 AM

Originally my intent was to ask the repository for the last execution - something like repository.getLastStepExecution(jobInstance, stepName). The same can be considered for last job execution and execution counts.

The benefit is it would remove mutual entity references. I guess one could argue this dumbs down the domain classes and shifts towards a 'smart' repository, but the repository has to be smart anyway as it needs to setup the entity references. As long as all classes asking for last executions and execution counts already have reference to repository this approach seems logical. Any opinions on this?

[ Permlink | « Hide ]

Dave Syer added a comment - 18/Feb/08 02:41 AM

I don't want to lose some of the domain references, but repository.getLastStepExecution(StepInstance) makes sense, and could easily be used by any Step implementation.

[ Permlink | « Hide ]

Robert Kasanicky added a comment - 20/Feb/08 03:26 AM

StepInstance has died.

[ Permlink | « Hide ]

Dave Syer added a comment - 07/Aug/08 10:06 AM

Assume closed as resolved and released