You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/07/19 13:35:24 UTC

[GitHub] [fineract] taskain7 opened a new pull request, #2437: Introducing Spring Batch as job runner engine

taskain7 opened a new pull request, #2437:
URL: https://github.com/apache/fineract/pull/2437

   ## Description
   
   Database tables for Spring Batch is created by Liquibase.
   Existing jobs have been moved to Spring Batch Tasklets.
   Only Spring Batch Manager isntance can execute jobs.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] galovics merged pull request #2437: [FINERACT-1667] Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
galovics merged PR #2437:
URL: https://github.com/apache/fineract/pull/2437


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] galovics commented on pull request #2437: [FINERACT-1667] Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
galovics commented on PR #2437:
URL: https://github.com/apache/fineract/pull/2437#issuecomment-1195163262

   @fynmanoj 
   
   > But there would be unintentional consequences to users who don’t want to be impacted by infra changes.
   
   Not sure about these "infra changes". Do you mind to elaborate?
   
   > Can you please integration test for one of the batch job honoring the job Params for batch size.
   
   Elaborate please. What batch size?
   
   > In the future, can similar pull requests be split into multiple pull request based on the feature it addresses, to make it easily reviewable
   
   That's always the goal but it's not possible for some cross cutting changes. Same for PostgreSQL support, I wasn't able to split it into smaller ones cause if I don't do it all the way through, builds are going to fail continuously. Same here. This is an all or nothing change.
   
   > Since it is a big change , I think we need to create 1.8.0 to separate from the cutting edge stuff going on develop, before this pull request get merged. To avoid large test cycle of 1.8.0 relese
   
   Respectfully disagree. If we start keeping changes on separate branches they will simply be not tested and forgotten/abandoned eventually and never getting merged. If we are lacking testing, let's work on improving those areas and fill the gaps.
   
   It's much better if we move forward with Fineract (or any product for that matter) instead of holding ourselves back from very valuable contributions from the community.
   
   My 2 cents.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] avikganguly01 commented on pull request #2437: [FINERACT-1667] Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
avikganguly01 commented on PR #2437:
URL: https://github.com/apache/fineract/pull/2437#issuecomment-1190248001

   Please hold this PR. Checking if it can cause any regression on interest posting jobs.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] galovics commented on pull request #2437: [FINERACT-1667] Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
galovics commented on PR #2437:
URL: https://github.com/apache/fineract/pull/2437#issuecomment-1193377835

   @avikganguly01 
   
   > Please hold this PR. Checking if it can cause any regression on interest posting jobs.
   
   I'm hoping you're not testing anything manually but you're running some automated tests (within Fineract)? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] galovics commented on a diff in pull request #2437: [FINERACT-1667] Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2437:
URL: https://github.com/apache/fineract/pull/2437#discussion_r928299457


##########
fineract-provider/src/main/java/org/apache/fineract/scheduledjobs/service/accountrunningbalanceupdate/AccountRunningBalanceUpdateConfig.java:
##########
@@ -0,0 +1,57 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.scheduledjobs.service.accountrunningbalanceupdate;
+
+import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
+import org.springframework.batch.core.Job;
+import org.springframework.batch.core.Step;
+import org.springframework.batch.core.configuration.annotation.EnableBatchProcessing;
+import org.springframework.batch.core.configuration.annotation.JobBuilderFactory;
+import org.springframework.batch.core.configuration.annotation.StepBuilderFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.jdbc.core.JdbcTemplate;
+
+@Configuration
+@EnableBatchProcessing
+public class AccountRunningBalanceUpdateConfig {
+
+    @Autowired
+    private JobBuilderFactory jobs;
+
+    @Autowired
+    private StepBuilderFactory steps;
+
+    @Bean
+    protected Step accountRunningBalanceUpdateStep(JdbcTemplate jdbcTemplate, DatabaseSpecificSQLGenerator sqlGenerator) {
+        return steps.get("accountRunningBalanceUpdate").tasklet(accountRunningBalanceUpdateTasklet(jdbcTemplate, sqlGenerator)).build();
+    }
+
+    @Bean
+    public Job accountRunningBalanceUpdateJob(JdbcTemplate jdbcTemplate, DatabaseSpecificSQLGenerator sqlGenerator) {
+        return jobs.get("taskletsJob").start(accountRunningBalanceUpdateStep(jdbcTemplate, sqlGenerator)).build();

Review Comment:
   Why do we call the job `taskletsJob`? It should be from the JobName enum for each and every job.



##########
fineract-provider/src/main/java/org/apache/fineract/scheduledjobs/service/accountrunningbalanceupdate/AccountRunningBalanceUpdateTasklet.java:
##########
@@ -0,0 +1,210 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.scheduledjobs.service.accountrunningbalanceupdate;
+
+import java.math.BigDecimal;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.accounting.common.AccountingEnumerations;
+import org.apache.fineract.accounting.glaccount.domain.GLAccountType;
+import org.apache.fineract.accounting.journalentry.data.JournalEntryData;
+import org.apache.fineract.accounting.journalentry.domain.JournalEntryType;
+import org.apache.fineract.infrastructure.core.data.EnumOptionData;
+import org.apache.fineract.infrastructure.core.domain.JdbcSupport;
+import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
+import org.springframework.batch.core.StepContribution;
+import org.springframework.batch.core.scope.context.ChunkContext;
+import org.springframework.batch.core.step.tasklet.Tasklet;
+import org.springframework.batch.repeat.RepeatStatus;
+import org.springframework.dao.EmptyResultDataAccessException;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.core.RowMapper;
+
+@Slf4j
+@RequiredArgsConstructor
+public class AccountRunningBalanceUpdateTasklet implements Tasklet {
+
+    private final JdbcTemplate jdbcTemplate;
+    private final DatabaseSpecificSQLGenerator sqlGenerator;
+    private final GLJournalEntryMapper entryMapper = new GLJournalEntryMapper();
+
+    @Override
+    public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception {
+        String dateFinder = "select MIN(je.entry_date) as entityDate from acc_gl_journal_entry  je "

Review Comment:
   Why did we copy the implementation from the `JournalEntryRunningBalanceUpdateServiceImpl`? In this case the Tasklet should be as simple as calling the original method. No copy paste please. :)



##########
fineract-provider/src/main/java/org/apache/fineract/scheduledjobs/service/accountrunningbalanceupdate/AccountRunningBalanceUpdateConfig.java:
##########
@@ -0,0 +1,57 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.scheduledjobs.service.accountrunningbalanceupdate;
+
+import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
+import org.springframework.batch.core.Job;
+import org.springframework.batch.core.Step;
+import org.springframework.batch.core.configuration.annotation.EnableBatchProcessing;
+import org.springframework.batch.core.configuration.annotation.JobBuilderFactory;
+import org.springframework.batch.core.configuration.annotation.StepBuilderFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.jdbc.core.JdbcTemplate;
+
+@Configuration
+@EnableBatchProcessing

Review Comment:
   This can appear once in the ServerApplication class.



##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/common/organisation/CampaignsTest.java:
##########
@@ -234,4 +234,11 @@ public void testSupportedActionsForCampaignWithError() {
         assertEquals(deletedCampaignId, campaignId);
 
     }
+
+    @Test
+    public void testUpdateSmsOutboundJobOutcome() throws InterruptedException {
+        Integer smsCampaign = campaignsHelper.createCampaign("Prospective Clients", 2);
+        String JobName = "Update SMS Outbound with Campaign Message";
+        // schedulerJobHelper.executeAndAwaitJob(JobName);

Review Comment:
   I assume this is just commented out by mistake. Please check.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/service/JobName.java:
##########
@@ -18,43 +18,111 @@
  */
 package org.apache.fineract.infrastructure.jobs.service;
 
+import org.apache.fineract.scheduledjobs.service.accountrunningbalanceupdate.AccountRunningBalanceUpdateTasklet;
+import org.apache.fineract.scheduledjobs.service.addaccrualentries.AddAccrualEntriesTasklet;
+import org.apache.fineract.scheduledjobs.service.addperiodicaccrualentries.AddPeriodicAccrualEntriesTasklet;
+import org.apache.fineract.scheduledjobs.service.addperiodicaccrualentriesforloanswithincomepostedastransactions.AddPeriodicAccrualEntriesForLoansWithIncomePostedAsTransactionsTasklet;
+import org.apache.fineract.scheduledjobs.service.applyannualfeeforsavings.ApplyAnnualFeeForSavingsTasklet;
+import org.apache.fineract.scheduledjobs.service.applychargetooverdueloaninstallment.ApplyChargeToOverdueLoanInstallmentTasklet;
+import org.apache.fineract.scheduledjobs.service.applyholidaystoloans.ApplyHolidaysToLoansTasklet;
+import org.apache.fineract.scheduledjobs.service.executeemail.ExecuteEmailTasklet;
+import org.apache.fineract.scheduledjobs.service.executereportmailingjobs.ExecuteReportMailingJobsTasklet;
+import org.apache.fineract.scheduledjobs.service.executestandinginstructions.ExecuteStandingInstructionsTasklet;
+import org.apache.fineract.scheduledjobs.service.generateadhocclientschhedule.GenerateAdhocClientScheduleTasklet;
+import org.apache.fineract.scheduledjobs.service.generateloanlossprovisioning.GenerateLoanlossProvisioningTasklet;
+import org.apache.fineract.scheduledjobs.service.generaterdschedule.GenerateRdScheduleTasklet;
+import org.apache.fineract.scheduledjobs.service.getdeliveryreportsfromsmsgateway.GetDeliveryReportsFromSmsGatewayTasklet;
+import org.apache.fineract.scheduledjobs.service.increasebusinessdateby1day.IncreaseBusinessDateBy1DayTasklet;
+import org.apache.fineract.scheduledjobs.service.increasecobdateby1day.IncreaseCobDateBy1DayTasklet;
+import org.apache.fineract.scheduledjobs.service.payduesavingscharges.PayDueSavingsChargesTasklet;
+import org.apache.fineract.scheduledjobs.service.postdividentsforshares.PostDividentsForSharesTasklet;
+import org.apache.fineract.scheduledjobs.service.postinterestforsavings.PostInterestForSavingTasklet;
+import org.apache.fineract.scheduledjobs.service.recalculateinterestforloan.RecalculateInterestForLoanTasklet;
+import org.apache.fineract.scheduledjobs.service.sendmessagetosmsgateway.SendMessageToSmsGatewayTasklet;
+import org.apache.fineract.scheduledjobs.service.transferfeechargeforloans.TransferFeeChargeForLoansTasklet;
+import org.apache.fineract.scheduledjobs.service.transferinteresttosavings.TransferInterestToSavingsTasklet;
+import org.apache.fineract.scheduledjobs.service.updatedepositsaccountmaturitydetails.UpdateDepositsAccountMaturityDetailsTasklet;
+import org.apache.fineract.scheduledjobs.service.updateemailoutboundwithcampaignmessage.UpdateEmailOutboundWithCampaignMessageTasklet;
+import org.apache.fineract.scheduledjobs.service.updateloanarrearsageing.UpdateLoanArrearsAgeingTasklet;
+import org.apache.fineract.scheduledjobs.service.updatenpa.UpdateNpaTasklet;
+import org.apache.fineract.scheduledjobs.service.updatesavingsdormantaccounts.UpdateSavingsDormantAccountsTasklet;
+import org.apache.fineract.scheduledjobs.service.updatesmsoutboundwithcampaignmessage.UpdateSmsOutboundWithCampaignMessageTasklet;
+import org.apache.fineract.scheduledjobs.service.updatetrialbalancedetails.UpdateTrialBalanceDetailsTasklet;
+import org.springframework.batch.core.step.tasklet.Tasklet;
+
 public enum JobName {
 
-    UPDATE_LOAN_ARREARS_AGEING("Update Loan Arrears Ageing"), APPLY_ANNUAL_FEE_FOR_SAVINGS(
-            "Apply Annual Fee For Savings"), APPLY_HOLIDAYS_TO_LOANS("Apply Holidays To Loans"), POST_INTEREST_FOR_SAVINGS(
-                    "Post Interest For Savings"), TRANSFER_FEE_CHARGE_FOR_LOANS(
-                            "Transfer Fee For Loans From Savings"), ACCOUNTING_RUNNING_BALANCE_UPDATE(
-                                    "Update Accounting Running Balances"), PAY_DUE_SAVINGS_CHARGES(
-                                            "Pay Due Savings Charges"), APPLY_CHARGE_TO_OVERDUE_LOAN_INSTALLMENT(
-                                                    "Apply penalty to overdue loans"), EXECUTE_STANDING_INSTRUCTIONS(
-                                                            "Execute Standing Instruction"), ADD_ACCRUAL_ENTRIES(
-                                                                    "Add Accrual Transactions"), UPDATE_NPA(
-                                                                            "Update Non Performing Assets"), UPDATE_DEPOSITS_ACCOUNT_MATURITY_DETAILS(
-                                                                                    "Update Deposit Accounts Maturity details"), TRANSFER_INTEREST_TO_SAVINGS(
-                                                                                            "Transfer Interest To Savings"), ADD_PERIODIC_ACCRUAL_ENTRIES(
-                                                                                                    "Add Periodic Accrual Transactions"), RECALCULATE_INTEREST_FOR_LOAN(
-                                                                                                            "Recalculate Interest For Loans"), GENERATE_RD_SCEHDULE(
-                                                                                                                    "Generate Mandatory Savings Schedule"), GENERATE_LOANLOSS_PROVISIONING(
-                                                                                                                            "Generate Loan Loss Provisioning"), POST_DIVIDENTS_FOR_SHARES(
-                                                                                                                                    "Post Dividends For Shares"), UPDATE_SAVINGS_DORMANT_ACCOUNTS(
-                                                                                                                                            "Update Savings Dormant Accounts"), ADD_PERIODIC_ACCRUAL_ENTRIES_FOR_LOANS_WITH_INCOME_POSTED_AS_TRANSACTIONS(
-                                                                                                                                                    "Add Accrual Transactions For Loans With Income Posted As Transactions"), EXECUTE_REPORT_MAILING_JOBS(
-                                                                                                                                                            "Execute Report Mailing Jobs"), UPDATE_SMS_OUTBOUND_WITH_CAMPAIGN_MESSAGE(
-                                                                                                                                                                    "Update SMS Outbound with Campaign Message"), SEND_MESSAGES_TO_SMS_GATEWAY(
-                                                                                                                                                                            "Send Messages to SMS Gateway"), GET_DELIVERY_REPORTS_FROM_SMS_GATEWAY(
-                                                                                                                                                                                    "Get Delivery Reports from SMS Gateway"), GENERATE_ADHOCCLIENT_SCEHDULE(
-                                                                                                                                                                                            "Generate AdhocClient Schedule"), UPDATE_EMAIL_OUTBOUND_WITH_CAMPAIGN_MESSAGE(
-                                                                                                                                                                                                    "Update Email Outbound with campaign message"), EXECUTE_EMAIL(
-                                                                                                                                                                                                            "Execute Email"), UPDATE_TRIAL_BALANCE_DETAILS(
-                                                                                                                                                                                                                    "Update Trial Balance Details"), EXECUTE_DIRTY_JOBS(
-                                                                                                                                                                                                                            "Execute All Dirty Jobs"), INCREASE_BUSINESS_DATE_BY_1_DAY(
-                                                                                                                                                                                                                                    "Increase Business Date by 1 day"), INCREASE_COB_DATE_BY_1_DAY(
-                                                                                                                                                                                                                                            "Increase COB Date by 1 day");
+    UPDATE_LOAN_ARREARS_AGEING("Update Loan Arrears Ageing", UpdateLoanArrearsAgeingTasklet.class), APPLY_ANNUAL_FEE_FOR_SAVINGS(

Review Comment:
   You can put a `//` to the end of every enum value line and that'll preserve the formatting for spotless.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] galovics commented on pull request #2437: [FINERACT-1667] Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
galovics commented on PR #2437:
URL: https://github.com/apache/fineract/pull/2437#issuecomment-1195298007

   @fynmanoj 
   
   > I mean the major framework level changes that can affect the users and deployments.
   
   I'm sorry but I'm still not sure what you mean. Spring Batch as a job runner engine doesn't affect any of the deployments and the change is backward compatible.
   
   > There are Job level configurations added to the jobs as job parameters. These include the batch size, thread pool etc. there are jobs that require these parameters. While adding a framework level change, It would be helpful to test the sanity of these configs.
   
   Good point and thanks for calling it out. @taskain7 please check this. For example the `JobName.RECALCULATE_INTEREST_FOR_LOAN` job has parameters.
   
   > In this PR other than the Batch changes, I could see some other changes like Sl4j related(in test cases mainly this is the only change.). Can't these be two different PRs?
   
   Agreed and that's unfortunate. Frankly speaking we could've definitely saved a couple 10-20 files to be touched by this refactoring but in the big picture, it wouldn't have made a huge difference.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] awasum commented on pull request #2437: Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
awasum commented on PR #2437:
URL: https://github.com/apache/fineract/pull/2437#issuecomment-1189401054

   Thanks for the beautiful contributions @taskain7 
   Please..is there a Fineract JIRA ticket for this one? I know it can be annoying but open source projects depend on them for continuity and transparency. Having a JIRA ticket describing the use case and issue you are solving will be a great idea. Also good idea to add unit/integration tests (it may already be there) and documentation on how to use this feature. Future users will be grateful for this and it will make it easy for others to contribute..extend this..even you one year from today will benefit from this. link to create issues: https://issues.apache.org/jira/projects/FINERACT/issues
   
   cc @adamsaghy , @galovics , @vidakovic , @ptuomola , @edcable , @fynmanoj , @avikganguly01 , @vorburger 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] fynmanoj commented on pull request #2437: [FINERACT-1667] Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
fynmanoj commented on PR #2437:
URL: https://github.com/apache/fineract/pull/2437#issuecomment-1195247727

   
   @galovics :
   Thanks for your response, let me try to clarify what I meant above.
   
   > By `"infra changes"` 
   I mean the major framework level changes that can affect the users and deployments. 
   
   > Elaborate please. What batch size?
   There are Job level configurations added to the jobs as job parameters. These include the batch size, thread pool etc. there are jobs that require these parameters. While adding a framework level change, It would be helpful to test the sanity of these configs.  
   
   In this PR other than the Batch changes, I could see some other changes like Sl4j related(in test cases mainly this is the only change.). Can't these be two different PRs?
   
   @vidakovic , I agree with you on the documentation part, IMO there should be a checkpoint before a major feature goes in.
   
    


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] taskain7 commented on pull request #2437: Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
taskain7 commented on PR #2437:
URL: https://github.com/apache/fineract/pull/2437#issuecomment-1189996070

   Hi Awasum,
   
   thank you for raising this topic, you are totally right. I’ve opened a JIRA ticket for this improvement, and I will rename my PR to title that contains the JIRA ticket number. Tests are not needed, because we have existing test for the jobs, and the jobs’ behavior have not changed at all. After my PR is merged, I will write an e-mail to the mail list about the changes, and Ascii Doc will be extended with Spring Batch soon.
   
   Sincerely,
   Norbert
   
   > 2022. júl. 19. dátummal, 20:10 időpontban Awasum Yannick ***@***.***> írta:
   > 
   > 
   > Thanks for the beautiful contributions @taskain7 <https://github.com/taskain7>
   > Please..is there a Fineract JIRA ticket for this one? I know it can be annoying but open source projects depend on them for continuity and transparency. Having a JIRA ticket describing the use case and issue you are solving will be a great idea. Also good idea to add unit/integration tests (it may already be there) and documentation on how to use this feature. Future users will be grateful for this and it will make it easy for others to contribute..extend this..even you one year from today will benefit from this. link to create issues: https://issues.apache.org/jira/projects/FINERACT/issues <https://issues.apache.org/jira/projects/FINERACT/issues>
   > cc @adamsaghy <https://github.com/adamsaghy> , @galovics <https://github.com/galovics> , @vidakovic <https://github.com/vidakovic> , @ptuomola <https://github.com/ptuomola> , @edcable <https://github.com/edcable> , @fynmanoj <https://github.com/fynmanoj> , @avikganguly01 <https://github.com/avikganguly01> , @vorburger <https://github.com/vorburger>
   > —
   > Reply to this email directly, view it on GitHub <https://github.com/apache/fineract/pull/2437#issuecomment-1189401054>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFBXE6F36KRQ4K673GQUCWTVU3VRJANCNFSM54AALHSQ>.
   > You are receiving this because you were mentioned.
   > 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] galovics commented on pull request #2437: [FINERACT-1667] Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
galovics commented on PR #2437:
URL: https://github.com/apache/fineract/pull/2437#issuecomment-1193377686

   @awasum That's my mistake. I think somehow I forgot to create a JIRA for this although I proposed this change on the devlist. The subject line was `Spring Batch integration` and here's the original content:
   ```
   Dear all,
   
   I'd like to introduce you to the idea of enhancing Fineract with the capability to handle high-volume batch jobs along with a few other things.
   
   As any financial system, Fineract also supports batch jobs but they're quite limited on the data volume it can handle. Currently a batch job is a single Java method with loading everything at once and processing that huge chunk of data in a single execution step/transaction.
   
   To make Fineract better, we need to alter this simplified framework a little bit and introduce something that's more battle-tested: the Spring Batch module is here to help.
   
   With introducing Spring Batch into the mix, we'll finally have a chance to decouple data reading, processing and data writing that'll finally bring us the benefit of making our batch jobs:
   - faster
   - reliable
   - being able to handle huge data volumes
   
   And I'm not gonna just stop there, with Spring Batch in place, we'll be able to distribute our batch job workloads into separate JVMs in a scenario where we want to run a cluster of Fineract instances to cope with the load.
   
   The next thing to the Spring Batch module integration is the introduction of a new batch job, Loan Close Of Business; that'll be the starting point of reworking the date handling in Fineract. This job will be simply responsible to "close" Loans on a daily basis making sure that interests/fees/etc are calculated properly. It's going to be the first full-fledged job that'll be able to run in a partitioned mode on a cluster of JVMs and multiple threads.
   
   The job will be rolled out as disabled by default so it won't interfere with any existing deployments but if somebody will need it's potential and business functionality, they can certainly enable it. Also, there's an extensive documentation I already wrote for this; right now it's in PR for some review and I'm gonna do a few additional modifications but you can check it out here: https://github.com/apache/fineract/pull/2326
   
   I don't want to repeat everything that's in the PR so if you're curious be sure to check it.
   
   TL;DR; Spring Batch is going to be integrated into Fineract. A new batch job called Loan Close Of Business will be created along with some additional functionality that's described in the PR above.
   
   Best,
   Arnold
   ```
   
   I apoligize for not creating the ticket and thanks for @taskain7 doing it on my behalf. :-)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] fynmanoj commented on pull request #2437: [FINERACT-1667] Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
fynmanoj commented on PR #2437:
URL: https://github.com/apache/fineract/pull/2437#issuecomment-1195090973

   1. Similar to how liquibase was not intended to change any data , this wont change the functionality of the batch job intentionally . But there would be unintentional consequences  to users who don’t want to be impacted by infra changes.
   2. Can you please integration test for one of the batch job honoring the job Params for batch size.
   3. In the future, can similar pull requests be split into multiple pull request based on the feature it addresses, to make it easily reviewable
   4. Since it is a big change , I think we need to create 1.8.0 to separate from the cutting edge stuff going on develop, before this pull request get merged. To avoid large test cycle of 1.8.0 relese
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] vidakovic commented on pull request #2437: [FINERACT-1667] Introducing Spring Batch as job runner engine

Posted by GitBox <gi...@apache.org>.
vidakovic commented on PR #2437:
URL: https://github.com/apache/fineract/pull/2437#issuecomment-1195183127

   ... I can see @fynmanoj 's point with big PRs, but agree with @galovics that some features just cannot be chopped up in smaller pieces.
   
   Concerning cutting a release before merging this here: let's take a step back and look at what we are replacing (Quartz jobs)... I think it is safe to say that literally everyone has issues with the existing Quartz jobs... for myself I can say that I have at least one conversation on jobs per week.... and usually it's nothing positive. So my take on this would be to have the earlier the better (so, not only because of things rotting away in branches... which is still a valid point).
   
   And yes, there might be something to take care of to migrate a production system to a release with batch feature, but that's a fair price to pay for such a major improvement... we just have to document things properly.
   
   2 more cents


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org