chore(xtest): adds otdf-sdk-mgr tool for otdf client binary mgmt#410
chore(xtest): adds otdf-sdk-mgr tool for otdf client binary mgmt#410dmihalcik-virtru wants to merge 16 commits intoopentdf:mainfrom
Conversation
Summary of ChangesHello @dmihalcik-virtru, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant overhaul of the OpenTDF testing and SDK management infrastructure. It centralizes SDK version resolution and test environment control into two new Python-based command-line interfaces: Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new Python packages, otdf-sdk-mgr and otdf-local, to manage SDK versions and the local test environment, which is a significant improvement over the previous shell scripts. The refactoring of existing scripts into wrappers for these new tools is well-executed. The code is well-structured, particularly with the use of typer and rich for the CLIs. I have identified a few inconsistencies in documentation paths and a minor bug in one of the CLI commands. Additionally, I've made a suggestion regarding the build configuration of one of the new packages. Overall, this is a solid contribution that will enhance the developer experience.
dmihalcik-virtru
force-pushed
the
feat/otdf-sdk-mgr
branch
from
bf78192 to
b00b2fe
Compare
dmihalcik-virtru
changed the title
dmihalcik-virtru
force-pushed
the
feat/otdf-sdk-mgr
branch
from
eb49663 to
bb2749a
Compare
dmihalcik-virtru
changed the title
- config: make SDK_DIR discovery lazy via get_sdk_dir()/get_sdk_dirs(); add OTDF_SDK_DIR env var override; no longer raises at import time - installers: replace sys.exit() with InstallError exception; fix partial-write by downloading to tempfile before moving into dist_dir; replace deprecated urlretrieve with urlopen + copyfileobj - resolve: guard LTS_VERSIONS lookup with explicit KeyError message - checkout: fix worktree update to use 'git -C <worktree> pull' instead of broken --git-dir/--work-tree combination - cli_install: make --sdk and --version truly required (typer.Option(...) with no default); catch InstallError in release/artifact commands - registry: add GITHUB_TOKEN auth header support for GitHub API requests; warn with rate-limit reset time on 403/429 responses - action.yaml: sanitize tag names for env var use (dots→underscores); add 'determine source checkout needs' step that checks both head==true and BUILD_FROM_SOURCE_<tag> fallback; update checkout step conditions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new Python-based CLI tool, otdf-sdk-mgr, to centralize and simplify the management of SDK versions for testing. The new tool handles version resolution, artifact installation from registries, and source code checkouts. Existing shell scripts have been refactored to be thin wrappers around this new tool, and CI workflows are updated accordingly. The changes are well-structured and significantly improve the maintainability of the SDK management process. My feedback includes suggestions to improve error handling by using exceptions instead of sys.exit for better testability and reusability, correcting a documentation inconsistency, and making exception handling more specific.
There was a problem hiding this comment.
Pull request overview
Adds a new otdf-sdk-mgr Python CLI to manage SDK version resolution, artifact installs, and source checkouts; updates xtest tooling and CI workflows to use it instead of bespoke shell/Python scripts.
Changes:
- Introduces
otdf-sdk-mgrpackage (Typer CLI) for version discovery/resolution, installs, cleanup, and Java post-checkout fixups. - Refactors
xtestscripts/docs and GitHub Action (setup-cli-tool) to prefer installing released artifacts and only checkout/build from source when needed. - Updates CI workflows to use
uv+otdf-sdk-mgrand adds linting for the new package.
Reviewed changes
Copilot reviewed 27 out of 30 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| xtest/test_audit_logs_integration.py | Updates run instructions to reflect new working directory. |
| xtest/setup-cli-tool/action.yaml | Adds artifact install + conditional source checkout logic using otdf-sdk-mgr. |
| xtest/sdk/scripts/requirements.txt | Removes GitPython requirement (migration to otdf-sdk-mgr). |
| xtest/sdk/scripts/post-checkout-java.sh | Deletes Java fixup shell script (replaced by Python). |
| xtest/sdk/scripts/cleanup-all.sh | Deletes cleanup shell script (replaced by CLI clean). |
| xtest/sdk/scripts/checkout-sdk-branch.sh | Deletes checkout helper script (replaced by CLI checkout). |
| xtest/sdk/scripts/checkout-all.sh | Deletes “checkout all” script (replaced by CLI checkout --all). |
| xtest/sdk/go/otdfctl.sh | Reads .version to pin go run version when no local binary exists. |
| xtest/sdk/go/cli.sh | Reads .version to pin go run version when no local binary exists. |
| xtest/README.md | Updates instructions to use otdf-sdk-mgr commands. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/semver.py | Adds lightweight semver parsing/sorting utilities. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/resolve.py | Refactors version resolution into a module and adds Java “latest with CLI” logic. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py | Adds registry querying for Go tags, npm, Maven, and GitHub Releases. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/java_fixup.py | Reimplements Java post-checkout pom fixups in Python. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py | Adds installers for Go/JS/Java CLI artifacts and install subcommands. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/config.py | Centralizes constants and discovers xtest/sdk directory. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py | Adds versions CLI commands (list/resolve). |
| otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py | Adds install CLI subcommands. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/cli.py | Adds top-level CLI and checkout/clean/java-fixup commands. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py | Implements bare-repo + worktree checkout logic. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/main.py | Enables python -m otdf_sdk_mgr. |
| otdf-sdk-mgr/src/otdf_sdk_mgr/init.py | Adds package metadata version. |
| otdf-sdk-mgr/pyproject.toml | Defines new Python project + deps + ruff/pyright config. |
| otdf-sdk-mgr/README.md | Documents CLI usage and install modes. |
| AGENTS.md | Documents how to configure SDK artifacts via otdf-sdk-mgr. |
| .github/workflows/xtest.yml | Uses uv + otdf-sdk-mgr for version resolution; adds conditional build steps. |
| .github/workflows/check.yml | Adds lint/typecheck steps for otdf-sdk-mgr. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix SonarCloud security hotspot: replace execSync with spawnSync in
xtest.yml to avoid command injection via template literal interpolation
- Use removeprefix("v") instead of lstrip("v") in java_fixup.py for
correct single-prefix stripping semantics
- Wrap urlopen HEAD check in context manager to prevent resource leak;
narrow broad except Exception to (URLError, OSError) in installers.py
- Replace sys.exit calls in checkout.py with proper exceptions
(CalledProcessError, ValueError); handle them in CLI entry point
- Remove redundant --json flag from cli_versions.py list command
- Fix shell word-splitting in setup-cli-tool/action.yaml by piping
through while read instead of for-in; improve tag_sanitized regex
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add new SDK management package for version resolution, artifact installation, and checkout. Refactor shell scripts to thin wrappers delegating to otdf-sdk-mgr. Update CI workflows with step IDs, conditional guards, and otdf-sdk-mgr version resolution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: David Mihalcik <dmihalcik@virtru.com>
- config: make SDK_DIR discovery lazy via get_sdk_dir()/get_sdk_dirs(); add OTDF_SDK_DIR env var override; no longer raises at import time - installers: replace sys.exit() with InstallError exception; fix partial-write by downloading to tempfile before moving into dist_dir; replace deprecated urlretrieve with urlopen + copyfileobj - resolve: guard LTS_VERSIONS lookup with explicit KeyError message - checkout: fix worktree update to use 'git -C <worktree> pull' instead of broken --git-dir/--work-tree combination - cli_install: make --sdk and --version truly required (typer.Option(...) with no default); catch InstallError in release/artifact commands - registry: add GITHUB_TOKEN auth header support for GitHub API requests; warn with rate-limit reset time on 403/429 responses - action.yaml: sanitize tag names for env var use (dots→underscores); add 'determine source checkout needs' step that checks both head==true and BUILD_FROM_SOURCE_<tag> fallback; update checkout step conditions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix SonarCloud security hotspot: replace execSync with spawnSync in
xtest.yml to avoid command injection via template literal interpolation
- Use removeprefix("v") instead of lstrip("v") in java_fixup.py for
correct single-prefix stripping semantics
- Wrap urlopen HEAD check in context manager to prevent resource leak;
narrow broad except Exception to (URLError, OSError) in installers.py
- Replace sys.exit calls in checkout.py with proper exceptions
(CalledProcessError, ValueError); handle them in CLI entry point
- Remove redundant --json flag from cli_versions.py list command
- Fix shell word-splitting in setup-cli-tool/action.yaml by piping
through while read instead of for-in; improve tag_sanitized regex
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dmihalcik-virtru
force-pushed
the
feat/otdf-sdk-mgr
branch
from
654cbf2 to
5ef4548
Compare
spawnSync does not perform shell word-splitting, so passing "main latest" as a single string only resolved one tag instead of both. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
1 participant
Summary
setup-cli-tool/action.yamlwith artifact installation supportotdf-sdk-mgrPython package for SDK version resolution, artifact installation, and checkoutotdf-sdk-mgrifguards for SDK build steps, andotdf-sdk-mgrversion resolutionParent PRs
feat/otdf-local)chore/add-shfmt)Test plan
cd otdf-sdk-mgr && uv sync && uv run ruff check . && uv run ruff format --check . && uv run pyrightshfmt -d .clean🤖 Generated with Claude Code
Part of stacked PR series decomposing
chore/the-claudiest-day-tmux