Skip to content
This repository was archived by the owner on May 14, 2025. It is now read-only.

Update JdbcSearchableJobExecutionDao to support Batch 4 and 5 schemas#5613

Merged
corneil merged 11 commits intospring-attic:mainfrom
corneil:GH-5609-job-configuration-location
Dec 22, 2023
Merged

Update JdbcSearchableJobExecutionDao to support Batch 4 and 5 schemas#5613
corneil merged 11 commits intospring-attic:mainfrom
corneil:GH-5609-job-configuration-location

Conversation

@corneil
Copy link
Contributor

@corneil corneil commented Dec 19, 2023

Update JdbcSearchableJobExecutionDao to support Batch 4 and 5 schemas because of the removal of JOB_CONFIGURATION_LOCATION.

Fixes #5609

@corneil corneil added this to the 2.11.2 milestone Dec 19, 2023
@corneil corneil requested a review from cppwfs December 19, 2023 07:28
@corneil corneil force-pushed the GH-5609-job-configuration-location branch from d973c28 to 3f6474a Compare December 19, 2023 08:53
Copy link
Contributor

@cppwfs cppwfs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just some nitpicks and we probably need to test the BATCH_5 paths. Good work!

@corneil corneil self-assigned this Dec 19, 2023
@ilayaperumalg ilayaperumalg assigned cppwfs and onobc and unassigned corneil Dec 19, 2023
… because of the removal of JOB_CONFIGURATION_LOCATION.

Fixes spring-attic#5609
@corneil corneil force-pushed the GH-5609-job-configuration-location branch from 3f6474a to 8e6a356 Compare December 19, 2023 17:37
@corneil corneil requested a review from cppwfs December 19, 2023 17:38
… because of the removal of JOB_CONFIGURATION_LOCATION.

Fixes spring-attic#5609
Copy link
Contributor

@onobc onobc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @corneil . I am marking as "requested changes" based on the need to add batch5 tests.

@corneil corneil requested a review from onobc December 20, 2023 12:41
…sts can be moved to AbstractSimpleJobServiceTests.
@corneil corneil marked this pull request as draft December 20, 2023 16:19
@corneil
Copy link
Contributor Author

corneil commented Dec 20, 2023

Working to combine all the tests from AbstractJdbcJobSearchableExecutionDaoTests into AbstractSimpleJobServiceTests.

…ctSimpleJobServiceTests.

Fix JobParameter loading on JdbcSearchableJobExecutionDao.
@corneil
Copy link
Contributor Author

corneil commented Dec 21, 2023

The PR build is failing with errors. The testcontainer is running out of connections.

Copy link
Contributor

@cppwfs cppwfs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the changes and only nitipicks. But the tests are failing locally on my machine and on the CI.

@cppwfs
Copy link
Contributor

cppwfs commented Dec 21, 2023

After the changes you Chris and I paired coding through, the tests look good. It LGTM after you resolve mine and Chris Nitpicks.

@corneil corneil marked this pull request as ready for review December 21, 2023 17:49
@corneil corneil requested review from cppwfs and onobc December 21, 2023 17:49
Copy link
Contributor

@onobc onobc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT - thanks @corneil .

@onobc onobc unassigned cppwfs and onobc Dec 21, 2023
@onobc onobc self-requested a review December 21, 2023 19:47
@corneil corneil merged commit 5073813 into spring-attic:main Dec 22, 2023
@corneil corneil deleted the GH-5609-job-configuration-location branch February 2, 2024 10:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Remove of JOB_CONFIGURATION_LOCATION broke some queries

3 participants