Skip to content

MLE-30316 Allow configurable socket timeout for classification requests in Flux#649

Merged
rjdew-progress merged 8 commits into
developfrom
MLE-30316
Jul 10, 2026
Merged

MLE-30316 Allow configurable socket timeout for classification requests in Flux#649
rjdew-progress merged 8 commits into
developfrom
MLE-30316

Conversation

@jonmille

@jonmille jonmille commented Jul 9, 2026

Copy link
Copy Markdown

Problem

The Semaphore classifier client had a hardcoded 10-second socket timeout with no way to override it. Classifying large or complex documents (e.g., PDFs with forced language settings) can exceed this limit, and there was no CLI option or Java API method available to raise it.

Solution

Added --classifier-timeout as a new CLI option and socketTimeout(int) as a new method on the ClassifierOptions Java API. The value is passed through to the Spark Connector as spark.marklogic.write.classifier.socketTimeout, where it is applied to the Semaphore ClassificationConfiguration.

When the option is not set, no value is forwarded to the connector and the Semaphore library's default of 10,000 ms is preserved — no behavior change for existing users.

Changes

ClassifierOptions.java (public API)

  • Added socketTimeout(int socketTimeoutMs) method (@since 2.1.2).

ClassifierParams.java

  • Added --classifier-timeout picocli option (Integer socketTimeoutMs, defaults to null so no entry is added to the options map when not specified).
  • Wired Options.WRITE_CLASSIFIER_SOCKET_TIMEOUT into makeOptions() via OptionsUtil.integerOption().
  • Implemented socketTimeout(int) from the ClassifierOptions interface.

ClassifierOptionsTest.java

  • testOptions() — updated to assert WRITE_CLASSIFIER_SOCKET_TIMEOUT is correctly mapped when socketTimeout(30000) is called.
  • defaultTimeoutNotIncludedWhenNotSet() — new test verifying the option key is absent from the options map when socketTimeout is never called.

CopyOptionsTest.java

  • Added --classifier-timeout 60000 to the copy command classifier options test and the corresponding assertOptions() assertion.

classifier.md

  • Added --classifier-timeout row to the options table.

Testing

Unit tests only — no MarkLogic instance required.

./gradlew :flux-cli:test --tests com.marklogic.flux.impl.importdata.ClassifierOptionsTest
./gradlew :flux-cli:test --tests com.marklogic.flux.impl.copy.CopyOptionsTest

All tests pass.

Notes

Jira Story: https://progresssoftware.atlassian.net/browse/MLE-30316

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a configurable socket timeout for Semaphore classifier requests so large/complex document classification can exceed the current fixed 10s default, while preserving existing behavior when the option isn’t set.

Changes:

  • Added ClassifierOptions.socketTimeout(int) to the Java API and implemented it in ClassifierParams.
  • Added a new CLI option --classifier-timeout and wired it through to Spark connector options as WRITE_CLASSIFIER_SOCKET_TIMEOUT.
  • Updated unit tests and documentation to cover and describe the new option.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
flux-cli/src/main/java/com/marklogic/flux/api/ClassifierOptions.java Adds the new public API method for configuring classifier socket timeout.
flux-cli/src/main/java/com/marklogic/flux/impl/importdata/ClassifierParams.java Adds the --classifier-timeout CLI flag and forwards the value into the Spark connector options map.
flux-cli/src/test/java/com/marklogic/flux/impl/importdata/ClassifierOptionsTest.java Extends tests to verify the new option is mapped, and verifies the default behavior (key absent when not set).
flux-cli/src/test/java/com/marklogic/flux/impl/copy/CopyOptionsTest.java Updates copy command option parsing/assertions to include the new classifier timeout option.
docs/import/classifier.md Documents --classifier-timeout in the classifier options table.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread flux-cli/src/main/java/com/marklogic/flux/impl/importdata/ClassifierParams.java Outdated
Comment thread docs/import/classifier.md Outdated
jonmille and others added 3 commits July 9, 2026 10:27
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@rjrudin rjrudin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would do seconds here as well and add a connection timeout, which means I would do --classifier-socket-timeout and --classifier-connection-timeout. The expectation is that the user is familiar with Semaphore and thus would be familiar with the two timeout settings that Semaphore provides.

Switched from milliseconds to seconds.
Renamed --classifier-timeout -> --classifier-socket-timeout
Added --classifier-connection-timeout
@jonmille jonmille requested a review from rjrudin July 9, 2026 17:30
rjrudin
rjrudin previously approved these changes Jul 9, 2026
Comment thread docs/import/classifier.md Outdated
| `--classifier-api-key` | Provide the API key for accessing the classifier service when hosted in Progress Data Cloud. |
| `--classifier-token-path` | Specify the path to the token generator for the classifier service when hosted in Progress Data Cloud. |
| `--classifier-batch-size` | Set the number of documents or text chunks to send in a single request to the classifier. Defaults to 20. |
| `--classifier-socket-timeout` | Set the socket timeout in seconds for classification requests. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I recommend including the default values here, as they're mentioned for other options above and I think a user would naturally wonder what they are and whether they should use something else.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added the defaults to the 2 new options

rjrudin
rjrudin previously approved these changes Jul 10, 2026
@rjdew-progress rjdew-progress merged commit a67875e into develop Jul 10, 2026
4 checks passed
@jonmille jonmille deleted the MLE-30316 branch July 10, 2026 14:56
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