Custom namespace for Job and related (Step, and maybe some *Reader/Writer) [BATCH-63]

spring-projects-issues commented 17 years ago

Dave Syer opened BATCH-63 and commented

StepScope needs to be validated - certain collaborators (like implementations of InputSource) have to be scope="step" and it is quite hard to validate. Lots of docos needed. A minimal custom namespace just for Tasklet configuration might be the most practical solution.

Affects: 1.0-m2

Attachments:

Batch Namespace - Spring Batch - SpringSource Wiki.webarchive (278.68 kB)
dsl-example-hibernate.xml (1.43 kB)
dsl-example-trades.xml (2.37 kB)
jobs.xml (1.64 kB)
spring-batch-1.0.m4.xsd (9.82 kB)
SpringBatch-6099862-20080218-16_00_28.pdf (175.81 kB)

spring-projects-issues commented 17 years ago

Dave Syer commented

Technical hitch - inner beans cannot be scoped...

This is turning into a documentation issue - no-one has time / technology to fix it properly.

spring-projects-issues commented 17 years ago

Dave Syer commented

The technical hitch is a bug SPR-3800 which should be fixed in 2.1-m4 (N.B. inner beans can be scoped - they were just not registering their destruction callbacks). But this whole issue is still not looking like it is going to be fixed for 1.0. Documentation covered elsewhere (BATCH-120).

spring-projects-issues commented 16 years ago

Lucas Ward commented

Dave,

Can you attach the example namespace we came up with last time I was in SH, for others to be able to review? I seem to have lost it in all my computer changes.

spring-projects-issues commented 16 years ago

Lucas Ward commented

I did a quick review of the schema that Ben attached recently. Everything looks good except for the database ItemReader. I see a Sql reader(should probably be jdbc reader), with a type of cursor or driving, and an iBatis reader. My issue is that the iBatis reader is really a driving reader. More specifically, it's a driving query reader with an iBatis generator. Normally, it wouldn't be too bad, but I think the difference between the two types is big enough to warrant a more explicit separation. This is because the cursor item reader returns the whole item, whereas it's generally expected that the driving one will only return the key, requiring more information to be loaded via another dao.

Everything else looks fine, although I'm curious how you plan to handle the column mappings in file input, since I don't see them in the prototype.

Lastly, one use case that needs to be maintained is the ability to set default settings for batch jobs that may be overridden. We currently do this by using Abstract beans with the default values, and child beans that can override them. I'm not sure how to best handle this with a namespace, but in my mind it's an extremely important use case to keep. If someone has even as few as 10 batch jobs, you wouldn't want to wire in exception handler (for logging out skipped records) separately for each job. it becomes even worse in the very common case where 50+ distinct batch jobs exist.

spring-projects-issues commented 16 years ago

nebhale commented

(In reply to comment #4)

I did a quick review of the schema that Ben attached recently. Everything looks good except for the database ItemReader. I see a Sql reader(should probably be jdbc reader)

We opted for SQL since from the perspective of the user, they don't actually deal with JDBC. This actually was discussed a bit and Dave and I both agree that SQL is more appropriate in this case.

, with a type of cursor or driving, and an iBatis reader. My issue is that the iBatis reader is really a driving reader. More specifically, it's a driving query reader with an iBatis generator. Normally, it wouldn't be too bad, but I think the difference between the two types is big enough to warrant a more explicit separation. This is because the cursor item reader returns the whole item, whereas it's generally expected that the driving one will only return the key, requiring more information to be loaded via another dao.

What kind of separation do you have in mind between a driving reader and a cursor reader? Basically from a configuration perspective, both of them require a query and then we take a look at the flag to determine which type of ItemReader to actually instantiate. The reason that iBatis is kept separately is because it doesn't actually require a query at all, it requires an iBatis named query. If you think this is still too confusing I'd be game for removing the \ tag completely as I think it's a pretty small use case. In the end the data being loaded by a separate DAO is actually just an implementation detail of your transformer rather than of the reader, so I'm pretty happy with the way this is configured now.

Everything else looks fine, although I'm curious how you plan to handle the column mappings in file input, since I don't see them in the prototype.

You're absolutely right on this. I do plan on letting this shake out as I actually do the implementations of those projects.

Lastly, one use case that needs to be maintained is the ability to set default settings for batch jobs that may be overridden. We currently do this by using Abstract beans with the default values, and child beans that can override them. I'm not sure how to best handle this with a namespace, but in my mind it's an extremely important use case to keep. If someone has even as few as 10 batch jobs, you wouldn't want to wire in exception handler (for logging out skipped records) separately for each job. it becomes even worse in the very common case where 50+ distinct batch jobs exist.

Well, I think that this is more of an issue in cases where we use very verbose configurations (like bean definitions). I think that simply having a ref to a bean name in a number of places is fine in the case. At the very least, I'd like to go to 1.0 without explicit support for abstract namespace elements and if in practice people dislike configuring each time (again, just a ref to a separate bean, not a full bean definition) then we can add it in. I'm very concerned about the backwards compatibility of the namespace, so I want to go with a minimal set of support in this case.

Dave, do you have any comments on any of this?

spring-projects-issues commented 16 years ago

nebhale commented

Working version of Spring Batch namespace. You'll need either Mac or Windows Safari to open it.

spring-projects-issues commented 16 years ago

Scott Wintermute commented

Ben, do you mind exporting the Spring Batch namespace as pdf for those of us who aren't using a Mac or Safari. I would prefer not to load another browser and Confluence should let you do an easy export of the page in PDF. Let me know if this is an issue.

spring-projects-issues commented 16 years ago

nebhale commented

The reason that I chose the other way is because the confluence export isn't very good. I've attached it though.

spring-projects-issues commented 16 years ago

Lucas Ward commented

We opted for SQL since from the perspective of the user, they don't actually deal with JDBC. This actually was discussed a bit and Dave and I both agree that SQL is more >appropriate in this case.

I disagree with this, since it's not used commonly in spring. For example, if you use JdbcTemplate, you don't have to work with Jdbc directly either, and certainly no more than you do with the JdbcCursorItemReader.

I also disagree with the comments about Driving vs. Cursor, there is a huge difference between the usage: Cursor returns an item, and driving returns a key (expecting you to create an item from it) It's a HUGE difference in usage. Furthermore, the difference between the two types should be understood by anyone wanting to read in from a database, as they could have a lot of impacts depending upon which database type you're using. I would much rather have two separate tags for driving and cursor than one with a 'switch' for the two. Another option may be just leaving ItemReaders out of the namespace for release 1.0.

Regarding 'abstract configuration. I have very strong feelings about this one, since we already have a lot of projects, some in production, that would be unable to use the namespace if support for this is not added. It is an extremely important use-case to allow projects to do things like set a default strategy for logging out skipped items (take a look at the chunkedStep for more details on that)

spring-projects-issues commented 16 years ago

Douglas C. Kaminsky commented

I don't see the value in including the infrastructure components (i.e. ItemReaders and ItemWriters) as schema elements at all as opposed to just using beans of the required type (enforced via the tools namespace). Here's my thinking:

The infrastructure components may change in code much faster than the schema can change. For example, if we release version 1.0.1 of the framework with an added flag for some item reader, we can't keep changing the published schema, thus more or less implicitly deprecating the tag.
More infrastructure components are bound to be added after the 1.0 release with no schema support. How do we decide what's important enough to create a tag for and what is not? It seems arbitrary.
We have to include support for a anyway to allow insertion of custom item readers / writers. Type safety is already addressable via the tools schema.
With the amount of configurable data for a given item reader or writer, a custom tag doesn't buy us much clarity - e.g. key generators for driving query readers.
We can always add it for a 1.5 or 2.0 release with an accompanying schema.

spring-projects-issues commented 16 years ago

Douglas C. Kaminsky commented

Just realized I posted before commenting on the abstraction issue - using abstract beans is EXTRAORDINARILY important for managing configuration. I just came off of a project where the projected number of batch jobs numbers in the thousands. Without the ability to abstract common aspects of the configurations, Spring Batch becomes exponentially less useful.

spring-projects-issues commented 16 years ago

nebhale commented

(In reply to comment #9)

I disagree with this, since it's not used commonly in spring. For example, if you use JdbcTemplate, you don't have to work with Jdbc directly either, and certainly no more than you do with the JdbcCursorItemReader.

This was actually a recommendation from Juergen himself. He said that from his perspective, it was an SQL only situation and he'd change it in Spring if he could. The first rev of the schema actually had data-source-reader there and when presented with jdbc-reader or sql-reader, he chose the latter.

I also disagree with the comments about Driving vs. Cursor, there is a huge difference between the usage: Cursor returns an item, and driving returns a key (expecting you to create an item from it) It's a HUGE difference in usage.

But this is an implementation detail. From the configuration perspective (and the left side of the chunk) the only thing that is required is a query and whether it is driving or not. When it comes to what is returned from the ItemReader, the code doesn't really care until after it's passed to the transformer. Since the transformer is written by the user, that detail is easily dealt with there. The configuration never has to care.

Another option may be just leaving ItemReaders out of the namespace for release 1.0.

This was considered for the 1.0 release, but as most of the configuration would then lie in the readers, we opted to have some sort of simplification.

Regarding 'abstract configuration. I have very strong feelings about this one, since we already have a lot of projects, some in production, that would be unable to use the namespace if support for this is not added.

The problem is of course that the concept of abstract namespace elements isn't really supported. The concept of the abstract bean is supported at the container level but a namespace element doesn't map one-to-one with a bean, so it doesn't really work out.

It is an extremely important use-case to allow projects to do things like set a default strategy for logging out skipped items (take a look at the chunkedStep for more details on that)

This I think gets more to the crux of the argument. What about the addition of a \ element that talks about default strategies rather than the concept of an abstract bean?

spring-projects-issues commented 16 years ago

Lucas Ward commented

Ben,

"This I think gets more to the crux of the argument. What about the addition of a \ element that talks about default strategies rather than the concept of an abstract bean?"

That is completely fine, I didn't mean that it had to support abstract beans, just that the ability to create default settings is an important use case. Being able to create a 'default step' is much much better actually.

@Sql vs. Jdbc I don't mind too much honestly, but if we're going to call the tag Sql instead of Jdbc, I would prefer to change the readers back to Sql* as well.

"But this is an implementation detail. From the configuration perspective (and the left side of the chunk) the only thing that is required is a query and whether it is driving or not. When it comes to what is returned from the ItemReader, the code doesn't really care until after it's passed to the transformer. Since the transformer is written by the user, that detail is easily dealt with there. The configuration never has to care."

I understand your point about the 'Writers' or 'Readers' dealing with the output of the readers as a separate concern from the configuration. However, my point is that there is a huge domain difference between the 'driving' and 'cursor' based readers. It's so important in fact, that I think it should be separated. I'm extremely concerned that someone will try and use a type of 'driving' but then use the same type of sql statement and RowMapper that they would use for cursor (i.e. for returning the item rather than a key) This would lead to every issue that Rob highlighted so well at TSE, since the DrivingQueryItemReader holds the keys in a list. I understand your point about trying to make them less verbose, but I'm not sure that trying to combine the different types in configuration is a good idea. I would instead prefer to keep them separated, as they are in the code by driving vs. cursor. and I would definitely like to have a '\ that has a \ tag. I know that documentation can help with some of these concerns, but I still feel like it's much better to be explicit.

spring-projects-issues commented 16 years ago

nebhale commented

(In reply to comment #10)

I don't see the value in including the infrastructure components (i.e. ItemReaders and ItemWriters) as schema elements at all as opposed to just using beans of the required type (enforced via the tools namespace). Here's my thinking:

This had actually been considered.

The infrastructure components may change in code much faster than the schema can change. For example, if we release version 1.0.1 of the framework with an added flag for some item reader, we can't keep changing the published schema, thus more or less implicitly deprecating the tag.

Well, we split the item-reader/writers off into another schema just so it could change quickly. It has been pointed out that a good factory could hide a lot of the complexity of the implementations thus negating the need for a namespace at all.

More infrastructure components are bound to be added after the 1.0 release with no schema support. How do we decide what's important enough to create a tag for and what is not? It seems arbitrary.

Well, I'd say it's driven by community demand. In one of the earlier comments I mentioned that I would not be averse to removing the ibatis support as it is a pretty small minority of users.

We have to include support for a anyway to allow insertion of custom item readers / writers. Type safety is already addressable via the tools schema.

We actually don't. We discussed having the ability to define an in-line bean, but to stay with the pattern set out by Spring, we were only going to allow references to other beans. In fact, Juergen pointed out that pretty much the only namespace that allows in-line bean definitions is 'beans'.

With the amount of configurable data for a given item reader or writer, a custom tag doesn't buy us much clarity - e.g. key generators for driving query readers.

Fair point. The issue is that with a namespace we can more clearly enumerate (and in some cases create a more concise definition for) the dependencies. That being said, I'm not against this.

We can always add it for a 1.5 or 2.0 release with an accompanying schema.

And that's why :)

spring-projects-issues commented 16 years ago

nebhale commented

(In reply to comment #11)

See Comment #12

spring-projects-issues commented 16 years ago

Douglas C. Kaminsky commented

I envisioned something to the effect of in the execution environment config file to address these concerns and more use of the \ tag in the actual job config to support inheritance.

spring-projects-issues commented 16 years ago

nebhale commented

(In reply to comment #13)

That is completely fine, I didn't mean that it had to support abstract beans, just that the ability to create default settings is an important use case. Being able to create a 'default step' is much much better actually.

We'll go with that then.

I'm extremely concerned that someone will try and use a type of 'driving' but then use the same type of sql statement and RowMapper that they would use for cursor (i.e. for returning the item rather than a key)

Couldn't the same argument be made that they would create a \ and then put in the cursor query? If the problem is a mistaken choice, I'm not sure that having separate elements is any more helpful.

spring-projects-issues commented 16 years ago

Wayne Lund commented

Lucas and Doug have already stated their positions really well but I wanted to weigh in on the issue. I would also prefer to see two separate tags for driving queries and cursor handling. I also agree with the issue of naming and would argue that Spring does the same thing. In other words, even though iBatis and jdbc both expose SQL the support classes are named after the implementing technology. I think we need to do the same. However, Doug highlights the problems of trying to be too detailed this release in our namespace.

I looked at the webarchive provided and have the same concerns that Doug expressed on including the detailed infrastructure items for the same reasons he's nicely pointed out. We do think there are going to need to be corrections on line items that include cursor | driving because they do behave quite differently. Probably sounds like we're ganging up on these three issues but our large batch client had 5500 jobs and an AbstractConfiguration would have been extremely helpful, although we didn't really have that notion at the time.

Summary:

1) I vote for naming conforming to technology 2) Provide an Abstract Configuration 3) Perhaps leave out the infrastructure components from the namespace until later since there seems to be a lot of discussion.

spring-projects-issues commented 16 years ago

nebhale commented

(In reply to comment #18)

I would also prefer to see two separate tags for driving queries and cursor handling.

I still haven't heard a compelling argument that two tags is actually less prone to error than a single tag. The same decision errors occur in both situations.

I also agree with the issue of naming and would argue that Spring does the same thing. In other words, even though iBatis and jdbc both expose SQL the support classes are named after the implementing technology. I think we need to do the same.

The chief architect of Spring feels we should go the other way, though... does that sway your opinion at all?

I looked at the webarchive provided and have the same concerns that Doug expressed on including the detailed infrastructure items for the same reasons he's nicely pointed out.

I'd like to point out that because we've broken the other namespace out (and heck, even if we hadn't) no one has to use the reader/writer namespace. It's there as a convince for elements that that are used in very common ways. There is actually no requirement that you define readers using the namespace, works just fine.

Probably sounds like we're ganging up on these three issues but our large batch client had 5500 jobs and an AbstractConfiguration would have been extremely helpful, although we didn't really have that notion at the time.

I'd like Dave to weigh in on this thread before I go changing it, but as I said earlier, I've got no problems with a default-step-configuration element.

spring-projects-issues commented 16 years ago

Douglas C. Kaminsky commented

Per previous comments, this is how I saw the job config schema looking -- note this is not intended to be a completed product, it was just a first stab at what I was personally thinking. The usage of the tool schema may also not be appropriate as I don't have much experience with it.

spring-projects-issues commented 16 years ago

Lucas Ward commented

It sounds like we're all in agreement with the 'Default-Step' that Ben mentioned. it's much betters solution than the abstract one anyway, since the Abstract/Parent bean definition approach was just a hack to support a 'default step'.

Re: Sql vs. Jdbc

Certainly, Juergen's opinion is extremely important, but I think my earlier point (along with wayne's) was that spring uses Jdbc*, so I would prefer to be consistent with spring proper, but I'm not opposed to calling it sql (I originally called it that when I named the classes) It's a fairly minor issue though. The only really important thing to me would be naming the actually classes behind the tags consistently (i.e. SqlCursorItemReader)

@ Driving vs. Cursor

Perhaps I erred when using the 'error prone' argument in the first place. I fundamentally have two issues with it:

1) There's a HUGE domain difference between the two. I still feel that that people should fundamentally understand the difference between the two. They should also understand that EVERY database item reader falls into one of two categories: Driving or Cursor, regardless of whether jdbc, hibernate, or iBatis is used on the backend.

2) The combination of configuration makes configuring the DrivingQuery readers confusing and weird. It works extremely well for the cursor input, but its very problematic for driving query since you probably don't need a row mapper, and there's some other areas where you may need row mappers when the driving query when you don't with cursor. In theory, someone could provide a rowmapper for the keys, and then a 'restart rowMapper' that knows how to convert them. I honestly need to do a bit of work on this, and there's a jira issue about it somewhere, but my point is generally that if you really dive into the various driving query item readers, you'll find a lot of these type of scenarios that aren't going to be covered gracefully by a single tag.

I would still very much prefer a separate driving query tag with a key generator tag:

\ \ kind of scratched up, but I think you can understand what I'm getting at.

spring-projects-issues commented 16 years ago

nebhale commented

(In reply to comment #20)

Per previous comments, this is how I saw the job config schema looking

This is actually what my first cut before the SpringSource meeting looked like pretty much. But to stay in line with the namespaces that are being created in spring, we decided to eliminate the in-line bean definitions. Looking at it now, there aren't really any significant uses of in-line bean definitions.

spring-projects-issues commented 16 years ago

Dave Syer commented

I don't care much about sql- vs jdbc-, and I also don't care if the class names are Jdbc and the XML is sql-.

The data source item readers are confusing (too many features), and would they benefit from a simpler, single-element config, even if that means we simplify the features? That is the tough question, it seems. Remember also that we don't have to cover all the available Java options in the namespace - I don't like to leave features out, and force people to use <beans:*/>, but it is an option (e.g. not to expose the restart row mapper in the multi-column case).

Why does a driving query return a primary key and a cursor a domain object (please don't call it an "item" - they are all items)? Is there any reason why both shouldn't do the same thing? Or why we can't go to press with at least an XML schema that is consistent between the two, and leaves more advanced features for beans: customizations.

spring-projects-issues commented 16 years ago

Lucas Ward commented

"I don't care much about sql- vs jdbc-, and I also don't care if the class names are Jdbc and the XML is sql-. "

I agree, it's a minor issue, but I would still like to be consistent. Let's do either Sql or Jdbc everywhere.

"Why does a driving query return a primary key and a cursor a domain object (please don't call it an "item" - they are all items)? Is there any reason why both shouldn't do the same thing? Or why we can't go to press with at least an XML schema that is consistent between the two, and leaves more advanced features for beans: customizations."

I originally tried to make the two be 'symmetrical' by having the driving query also return the 'details' about each item. However, if you think it's complex now, it would be 4x so if we tried to do that. Furthermore, the details are generally very domain specific and likely already have a dao for them. I also had a lot of projects that were forgoing it and letting developers just wire a dao in. It also makes a lot of sense if you think about an asynchronous processing case. It would be much more efficient to give a chunk that's full of keys to be written, rather than reading back the rest of the details on the client side, which takes time, and passing that out.

I understand your point about trying to simplify it. Afterall, without any attempt at simplification there's honestly very little value to the tags. However, I think it's too much confusion for too little simplification.

spring-projects-issues commented 16 years ago

Lucas Ward commented

I wanted to add another comment about the namespace as well. The following is a bean definition for the JdbcCursorItemReader with the beans namespace:

<bean id="customerSqlItemReader" class="org.springframework.batch.io.cursor.JdbcCursorItemReader"
    scope="step" p:dataSource-ref="dataSource" p:sql="SELECT id, name, credit FROM customer" p:mapper-ref="creditMapper">

with the proposed namespace it would be:

\ \SELECT id, name, credit FROM customer\ \ In this case, I would argue there is very little value to the namespace.

spring-projects-issues commented 16 years ago

Dave Syer commented

After some chats with Lucas...

I think I sort of agree - there is much to risk by putting out the item/infrastructure namespace too early, and not much to be gained (most of the encapsulation we are looking for is in the step and job implementations).

Maybe we should can the idea of an infrastructure namespace and put more effort into JobLauncher (which is pretty simple, but ubiquitously used?)

spring-projects-issues commented 16 years ago

nebhale commented

(In reply to comment #25)

In this case, I would argue there is very little value to the namespace.

Remember that often the value of a namespace has very little do with actual XML reduction; in many cases that is simply a side-effect. The real value of a namespace typically lies in the domain specificity of the declaration. Talking about the domain elements rather than properties of Java classes is what a namespace buys you. In addition there is value is the development time validation that schema validation gives you. With out use of setters everywhere for properties, it's nearly impossible to get any useful validation before run/test-time.

spring-projects-issues commented 16 years ago

nebhale commented

I think I sort of agree - there is much to risk by putting out the item/infrastructure namespace too early, and not much to be gained (most of the encapsulation we are looking for is in the step and job implementations).

We had talked about making sure that we actually injected factories into the step's rather than ItemReader(Writer)s. Could we get a similar effect to a namespace where we create an ItemReader(Writer)Factory interface that simplifies all of the wiring for us? I'd be totally OK if we went that way and eliminated the Item namespace.

Maybe we should can the idea of an infrastructure namespace and put more effort into JobLauncher (which is pretty simple, but ubiquitously used?)

Sounds like a plan contingent on the previous. I'll start working that way now.

spring-projects-issues commented 16 years ago

Lucas Ward commented

Ben,

Regarding the value of a namespace, I understand your point that it's not just for 'line reduction', and has other values, but I guess Dave said it better than I: the disadvantages outweigh the advantages right now.

@ItemReader Factories I couldn't agree with you more, and it's something I've been discussing with Dave for awhile. If you look at the ItemStream issue, I'm starting to wonder if the Chunker and Dechunker can't play the role of factory, leaving the stream manager to manage only transactions and Execution context storage/retrieval. I've already had issues with the ChunkedStep having to turn the chunker into a split version of the Tasklet that still delegates with a load of instanceof checks.

It sounds like from your last comment that you're abandoning the infrastructure tags and creating some tags to help with the JobLauncher (and I assume the repo as well). What's the status on this? I haven't seen any commits go through, so I was curious.

spring-projects-issues commented 16 years ago

Dave Syer commented

this is relevant to the ongoing discussion in BATCH-364. In my opinion Chunker cannot be as good a gerenal purpose factory for ItemReaders as the BeanFactory. With step scope we get the best object factory in the business, and no need for every implementation of ItemReader to have its own factory implementation.

spring-projects-issues commented 16 years ago

Lucas Ward commented

Ben,

I've seen a couple of updates go by, what's the current status of the namespace?

spring-projects-issues commented 16 years ago

nebhale commented

The \ work is done, the \ stuff is done. Working on \ (we decided to go with that instead of \) and \. I haven't forgotten about the default step configuration stuff, but I need to talk with Juergen about the best way to do it.

spring-projects-issues commented 16 years ago

nebhale commented

Moving this to the sandbox for future reference and possible implementation...

spring-projects-issues commented 16 years ago

Dave Syer commented

Re-opened for 2.0

spring-projects-issues commented 16 years ago

Dave Syer commented

Attached updated XML examples

spring-projects / spring-batch

Custom namespace for Job and related (Step, and maybe some *Reader/Writer) [BATCH-63] #3515