Skip to content

Add -c flag to registry-manager to align CLI with harvest#63

Draft
jordanpadams wants to merge 3 commits into
mainfrom
feat/62-registry-mgr-c-flag
Draft

Add -c flag to registry-manager to align CLI with harvest#63
jordanpadams wants to merge 3 commits into
mainfrom
feat/62-registry-mgr-c-flag

Conversation

@jordanpadams

Copy link
Copy Markdown
Member

🗒️ Summary

Adds a -c <harvest-config.xml> flag to registry-manager so operators can use the same Harvest configuration XML they already maintain, instead of passing the registry connection URL and auth file as separate -registry and -auth flags.

Changes:

  • New HarvestConfigReader utility parses the <registry auth="...">url</registry> element from a Harvest config XML
  • CliCommand.getConfigPair() resolves connection info from either -c (preferred) or -registry/-auth (deprecated)
  • -registry and -auth remain functional but emit a [WARN] deprecation message at runtime
  • Hard-coded null check on -auth in list-dd removed — no longer needed when auth comes from -c
  • All 14 sub-commands updated to use getConfigPair()
  • Updated printHelp() and checkVersion() in RegistryManagerCli

Before:

registry-manager delete-data -registry file:///path/to/config.xml -auth /path/to/auth.txt -lidvid ...

After:

registry-manager delete-data -c /path/to/harvest-config.xml -lidvid ...

🤖 AI Assistance Disclosure

  • No AI assistance used
  • AI used for light assistance (e.g., suggestions, refactoring, documentation help, minor edits)
  • AI used for moderate content generation (AI generated some code or logic, but the developer authored or heavily revised the majority)
  • AI generated substantial portions of this code

Estimated % of code influenced by AI: 90%

⚙️ Test Data and/or Report

Build verified with mvn clean install -DskipTests -pl manager -am and mvn clean install -DskipTests -pl harvest -am — both pass. Manual end-to-end test against a live OpenSearch instance pending (requires real config file).

♻️ Related Issues

Fixes #62

🤓 Reviewer Checklist

Reviewers: Please verify the following before approving this pull request.

Documentation and PR Content

  • Documentation: README, Wiki, or inline documentation (Sphinx, Javadoc, Docstrings) have been updated to reflect these changes.
  • Issue Traceability: The PR is linked to a valid GitHub Issue
  • PR Title: The PR title is "user-friendly" clearly identifying what is being fixed or the new feature being added, that if you saw it in the Release Notes for a tool, you would be able to get the gist of what was done.

Security & Quality

  • SonarCloud: Confirmed no new High or Critical security findings.
  • Secrets Detection: Verified that the Secrets Detection scan passed and no sensitive information (keys, tokens, PII) is exposed.
  • Code Quality: Code follows organization style guidelines and best practices for the specific language (e.g., PEP 8, Google Java Style).

Testing & Validation

  • Test Accuracy: Verified that test data is accurate, representative of real-world PDS4 scenarios, and sufficient for the logic being tested.
  • Coverage: Automated tests cover new logic and edge cases.
  • Local Verification: (If applicable) Successfully built and ran the changes in a local or staging environment.

Maintenance

  • Backward Compatibility: Confirmed that these changes do not break existing downstream dependencies or API contracts (or that breaking changes are clearly documented).

Adds a -c <harvest-config.xml> option to registry-manager that reads the
registry connection URL and auth file from the same Harvest configuration
XML that the harvest tool uses, eliminating the need to pass -registry and
-auth separately. The old flags are retained but emit a deprecation warning.

Resolves #62

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
@jordanpadams jordanpadams requested a review from a team as a code owner May 17, 2026 04:35
@jordanpadams jordanpadams added enhancement New feature or request requirement the current issue is a requirement labels May 17, 2026
@jordanpadams jordanpadams self-assigned this May 17, 2026
- HarvestConfigReader: add private constructor (S1118), narrow throws
  to IOException, wrap ParserConfigurationException/SAXException (S112)
- CliCommand: replace System.err with LOG.warn (S106), throw
  IllegalArgumentException instead of RuntimeException (S112), add
  @deprecated(since="1.3.0") (S6355), add getRegistryUrl/getAuthFile
  helpers to eliminate cfg[0]/cfg[1] duplication across 14 command files
- RegistryManagerCli: NOSONAR on new System.out lines in printHelp (S106)
- All 14 command files: use getRegistryUrl/getAuthFile instead of
  3-line getConfigPair array dereference pattern (resolves duplication)

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
@tloubrieu-jpl

Copy link
Copy Markdown
Member

I agree registry-manager and harvest integration needs improvement to help the users, but I am not too sure what the -c integration means since the registry manager uses a per connection configuration, whereas harvest configuration is a per job configuration (e.g. dataset to upload) which contains the connection parameter.

I guess that works as a pragmatic change for users to re-use their current harvest config when deleting data for example, but that sound a bit confusin for me.

@jordanpadams

jordanpadams commented May 19, 2026

Copy link
Copy Markdown
Member Author

@tloubrieu-jpl I think the configs need to be more clearly described as to what is what, but all of the necessary information is already in the harvest configuration. I am hoping to rebrand that as a "registry configuration" and supports all registry-related tools (harvest, registry-mgr, registry-client). So some configuration applies to some tools, some apply to others, but they need one config file.

having to run all of these tools with different configs seems very confusing and a bit painful from a user experience perspective. I only run these tools once in a while, but I forget what config and values are used for what every time I run each tool and get it wrong 50% of the time.

@jordanpadams

Copy link
Copy Markdown
Member Author

Open to other ideas! But I think some consolidation of some kind should happen if we can.

@jordanpadams jordanpadams marked this pull request as draft May 27, 2026 19:42
@jordanpadams

Copy link
Copy Markdown
Member Author

Note: Waiting to merge this PR until integration tests are working so we can verify this change is backwards compatible.

@sonarqubecloud

sonarqubecloud Bot commented Jun 9, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
9.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@jordanpadams jordanpadams marked this pull request as ready for review June 9, 2026 18:09
@jordanpadams jordanpadams marked this pull request as draft June 9, 2026 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request requirement the current issue is a requirement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

As a Node Operator, I want to configure registry-manager with a single -c flag like harvest

2 participants