Skip to content

Conversation

@rjrudin
Copy link
Contributor

@rjrudin rjrudin commented Oct 20, 2025

Using new ml-gradle task

Copilot AI review requested due to automatic review settings October 20, 2025 15:48
@github-actions
Copy link

github-actions bot commented Oct 20, 2025

Copyright Validation Results
Total: 2 | Passed: 0 | Failed: 0 | Skipped: 2 | at: 2025-10-21 17:00:12 UTC | commit: fa34941

⏭️ Skipped (Excluded) Files

  • Jenkinsfile
  • test-app/build.gradle

✅ All files have valid copyright headers!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces a fixed 90-second sleep with a dynamic wait function that polls MarkLogic's readiness endpoint, allowing the pipeline to proceed as soon as MarkLogic is ready (up to 2 minutes maximum). The change reduces unnecessary wait time while providing better visibility through request logging.

Key changes:

  • Removed fixed 90-second sleep after Docker Compose startup
  • Added waitUntilMarkLogicIsReady() function that polls the /v1/ping endpoint with 5-second intervals
  • Applied the wait function to all four pipeline stages (ML 12 main, ML 11, ML 12 nightly, ML 10 nightly)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Jenkinsfile Outdated
Comment on lines 59 to 72
script {
timeout(time: 120, unit: 'SECONDS') {
waitUntil(initialRecurrencePeriod: 5000) {
try {
sh 'echo "Checking if MarkLogic is ready..."'
sh 'curl -i --anyauth --user admin:admin -X GET http://localhost:8000/v1/ping'
return true
} catch(exception){
return false
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Hardcoded credentials 'admin:admin' are exposed in plain text. Consider using Jenkins credentials binding to securely inject authentication credentials.

Suggested change
script {
timeout(time: 120, unit: 'SECONDS') {
waitUntil(initialRecurrencePeriod: 5000) {
try {
sh 'echo "Checking if MarkLogic is ready..."'
sh 'curl -i --anyauth --user admin:admin -X GET http://localhost:8000/v1/ping'
return true
} catch(exception){
return false
withCredentials([usernamePassword(credentialsId: 'ml-admin-credentials', usernameVariable: 'ML_USERNAME', passwordVariable: 'ML_PASSWORD')]) {
script {
timeout(time: 120, unit: 'SECONDS') {
waitUntil(initialRecurrencePeriod: 5000) {
try {
sh 'echo "Checking if MarkLogic is ready..."'
sh 'curl -i --anyauth --user ${ML_USERNAME}:${ML_PASSWORD} -X GET http://localhost:8000/v1/ping'
return true
} catch(exception){
return false
}

Copilot uses AI. Check for mistakes.
@rjrudin rjrudin force-pushed the feature/24763-sleep branch 2 times, most recently from ce728af to 9e6ccf6 Compare October 20, 2025 17:00
@rjrudin
Copy link
Contributor Author

rjrudin commented Oct 20, 2025

@SameeraPriyathamTadikonda Does this look okay? I took your function from the ragplus repository and modified to look for a 204 No Content.

stevebio
stevebio previously approved these changes Oct 20, 2025
Copy link
Collaborator

@stevebio stevebio left a comment

Choose a reason for hiding this comment

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

think this is much better.

@rjrudin rjrudin force-pushed the feature/24763-sleep branch 2 times, most recently from c8715c4 to 2e42fb9 Compare October 21, 2025 13:54
Using new ml-gradle task
@rjrudin rjrudin force-pushed the feature/24763-sleep branch from 2e42fb9 to fa34941 Compare October 21, 2025 16:59
Copy link
Contributor

@BillFarber BillFarber left a comment

Choose a reason for hiding this comment

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

Pending the current develop branch build.

@rjrudin
Copy link
Contributor Author

rjrudin commented Oct 21, 2025

Going to reopen this

@rjrudin rjrudin closed this Oct 21, 2025
@rjrudin rjrudin deleted the feature/24763-sleep branch October 21, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants