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

Group JDBC-related batch properties beneath spring.batch.jdbc #25053

Closed
fmbenhassine opened this issue Jan 29, 2021 · 5 comments
Closed

Group JDBC-related batch properties beneath spring.batch.jdbc #25053

fmbenhassine opened this issue Jan 29, 2021 · 5 comments
Assignees
Labels
status: superseded An issue that has been superseded by another type: enhancement A general enhancement

Comments

@fmbenhassine
Copy link
Contributor

As of v2.4.2, JDBC-related properties for batch are under spring.batch, unlike quartz, session and integration where those properties are under spring.xxx.jdbc.

After discussing this with @wilkinsona , I understand that this is because JDBC is currently the only store option for batch. However, I think it would be better to group those properties beneath spring.batch.jdbc for 1) naming consistency and 2) to pave the way for other storage options for batch in the future (We have indeed a feature request to support a non-jdbc job repository implementation, see spring-projects/spring-batch#877 which we might consider in the next major release).

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 29, 2021
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 29, 2021
@wilkinsona wilkinsona added this to the 2.x milestone Jan 29, 2021
@chaundhyan
Copy link
Contributor

chaundhyan commented Feb 2, 2021

@benas, @wilkinsona
If allowed, may I take this enhancement ahead, please?
If you agree, please assign this to me.

Thanks

@wilkinsona
Copy link
Member

wilkinsona commented Feb 2, 2021

Thanks for the offer, @chaundhyan. That would be great. We'll need to deprecate the existing properties (using @DeprecatedConfigurationProperty) and then provide replacements in the new spring.batch.jdbc. location. Please let us know if you have any questions.

@chaundhyan
Copy link
Contributor

@benas @wilkinsona : I've had done the changes. Would you like to review it over my repo?
My Change Comparison

Shall i raise the PR, if yes then into which branch?
I've had taken a fork and pushed changes into main branch over my forked repo.

If any suggestion, please do let me know.

Also there exists 3 test cases which are failing while doing ./gradlew build. But they are passing individually.
JettyReactiveWebServerFactoryTests. whenServerIsShuttingDownGracefullyThenNewConnectionsCannotBeMade()
org.springframework.web.reactive.function.client.WebClient was not fulfilled within 30 seconds.

Same way other are failing.
NettyReactiveWebServerFactoryTests. whenServerIsShuttingDownGracefullyThenNewConnectionsCannotBeMade()
TomcatReactiveWebServerFactoryTests. whenServerIsShuttingDownGracefullyThenNewConnectionsCannotBeMade()

Please suggest.

Thanks
Mukul Kumar Chaundhyan

@wilkinsona
Copy link
Member

wilkinsona commented Feb 9, 2021

@chaundhyan Yes, please do raise a PR against the master branch. One change that I would make to your proposal is to nest BatchJdbcProperties inside BatchProperties and rename it to Jdbc. BatchProperties should then have a Jdbc jdbc = new Jdbc() field and a getter method.

@snicoll
Copy link
Member

snicoll commented Feb 17, 2021

Closing in favor of PR #25316

@snicoll snicoll closed this as completed Feb 17, 2021
@snicoll snicoll removed this from the 2.x milestone Feb 17, 2021
@snicoll snicoll added the status: superseded An issue that has been superseded by another label Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants