Skip to content

Conversation

@b-long
Copy link
Owner

@b-long b-long commented Sep 10, 2025

No description provided.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @b-long, 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 streamlines the SDK initialization logic within the cmd_inspect function. The change removes a fallback mechanism that previously allowed the inspection command to proceed with a minimal, unauthenticated SDK even if authentication failed. This simplifies the code path and ensures that inspection operations are always performed with a fully authenticated SDK.

Highlights

  • SDK Initialization Simplification: The cmd_inspect function in src/otdf_python/cli.py no longer attempts to build a minimal, unauthenticated SDK if the primary authenticated SDK build fails. It now strictly requires a successful SDK build for inspection.
Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request cleans up the cmd_inspect function by removing a complex try-except block. While this simplifies the code, it also appears to remove the ability to perform an unauthenticated inspection of a TDF manifest. The previous implementation allowed for creating a minimal SDK to view the manifest even if authentication failed, which was a useful feature. The new implementation falls back to a very basic inspection, which is a functional regression. My review includes a comment detailing this concern.

Comment on lines 358 to -372
try:
try:
sdk = build_sdk(args)
except CLIError as auth_error:
# If authentication fails, create minimal SDK for basic inspection
logger.warning(f"Authentication failed, using minimal SDK: {auth_error}")
builder = SDKBuilder()
if args.platform_url:
builder.set_platform_endpoint(args.platform_url)
if hasattr(args, "plaintext") and args.plaintext:
builder.use_insecure_plaintext_connection(True)
if args.insecure:
builder.use_insecure_skip_verify(True)
sdk = builder.build()
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This change removes the logic for creating a 'minimal' SDK for unauthenticated inspection, which appears to be a functional regression. Previously, it was possible to inspect a TDF's manifest without providing authentication credentials, as the code would fall back to a minimal SDK capable of parsing the TDF structure. With this change, any authentication failure during SDK building results in a very basic inspection that only reports file type and size, losing the valuable ability to view the manifest without full credentials. If retaining unauthenticated manifest inspection is desired, the previous logic should be restored, possibly in a more robust implementation.

@b-long b-long merged commit 13be279 into develop Sep 10, 2025
9 checks passed
@b-long b-long deleted the cleanup-and-release branch September 10, 2025 19:41
b-long added a commit that referenced this pull request Sep 11, 2025
* Begin rewrite in pure Python

* Organize: git mv src/otdf_python/test_*.py tests/

* Format according to 'ruff'

* Fix static analysis

* Cleanup and organize tests/test_validate_otdf_python.py

* Remove 'TDFConfig' type from 'otdf_python.tdf'

* Fix description & formatting

* Add 'pydantic-settings' to dev & update dependencies

* Correct version number

* Cleanup and fix OIDC tests

* Comment old style integration test

* Execute majority of tests

* Allow import from 'tests'

* Fix string encryption test

* Remove dead code

* Adjust integration test

* Remove old build scripts

* Update README

* Update GHA triggers

* Fix endpoint URL and TLS verification

* ✅ Significant update 143 out of 150 tests passing

- When run with the proper .env file: 7 failed, 142 passed, 2 skipped, 1 warning
- Critical naming fix
- Update .proto files
- Add script to update .proto files
- Ditch HTTP impl
- Improve manifest and encrypt test
- Python CLI decrypt now works correctly with TDF files created by otdfctl

* Run all tests, except integration

* Update GHA configuration

* Mark integration tests

* Fix mocked tests/test_kas_client.py

* Mark integration tests

* Only build for 3.13 (temporary)

* Update license

* Enable and fix integration tests in CI

Cleanup tests

* Improve support for plaintext

* Make log collection optional

* Fix tests for plaintext

* Fix docstrings

* Fix docstrings

* Extract Connect RPC class

* Fix additional roundtrip testing

* Fix tests after kas_client updates

* Expand KAS client integration tests

* Fix mimeType

* Expand testing, fix compression bug

* Auto-use check_for_otdfctl fixture

* Expand static analysis, fix FURB188

* Use 'NULL_POLICY_UUID' for now

* Update kas_client.py & tdf.py, expand tests

* Expand & organize integration tests

* Expand static analysis, fix PT018

* Use configurable attrs in testing

* Use configurable attrs in testing

* Examine entitlements in CI

* Extract 'temp_credentials_file' fixture

* Rename file

* Modernize release workflows

* Modernize release workflows

* Update release workflow

* Manage 'otdf-python-proto' as a sub-package

* Update README

* Manage 'otdf-python-proto' as a sub-package

* Support Python 3.10+

* Fix version number

* Fix Python version requirement

* Bump version 0.3.0a4 -> 0.3.0a5

* Fix version extract command

* Undo file name change

* More support for PE flows, cleanup & improved typing (#70)

* Cleanup & improved typing

* Disable odd policy enforcement

* Add ".env-docker" file for local testing

* Add PE test support (GHA and docker) (#71)

* Add docker start script

* Gemini fixes

* Update GHA configuration

* Gemini fixes

* Enable PE e2e test

* Run 'pre-commit autoupdate' & fix lint issues

* Extract '_get_sdk_builder' function

* Cleanup & remove redundant function

* Improve typing

* Use patch() context manager, reduce imports

* Remove unnecessary import

* Combine 'yq' expressions

* Point to commit SHA

* Remove hallucination

* Match version number

* Bump 0.3.0a5 to 0.3.0a6

* Chore/update docs and release process (#72)

* Cleanup docs

* Refine workflows for release management and testing

- Implement `release-please` workflow for automated releases.
- Create `publish-test` and `publish` workflows to handle package builds and releases.
- Introduce `test-suite` workflow to run tests before publishing.
- Update configuration files for release management.

* Add 'ruff' as dev dependency

* Configure ruff to ignore generated files

* Fail fast if linting fails

* Document release process

* Bump version to 0.3.0a7

* Publish new alpha

* Allow replacing artifacts with the same name

* Remove the duplicate integration-test job

* Attempt alpha release

* chore: improve pre-commit configuration

* chore: revert 'rm CONNECT_RPC_MIGRATION.md'

* chore: disable TestPyPIBuild unless workflow_dispatch

* chore: bump version 0.3.0a7 -> 0.3.0a8

* chore: bump version 0.3.0a8 -> 0.3.0a9

* chore: target this branch

* chore: target develop branch

* chore: fix release-please config

* chore: fix version number

* chore: use standard 'workflow_call'

* chore: clean up publishing

* fix: fix publishing

* chore: release 0.3.0a10

Release-As: 0.3.0a10

* fix: fix publishing

* chore: release 0.3.0a11

Release-As: 0.3.0a11

* chore: release develop (#81)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* chore: align version numbers

* chore: add 'otdf-python-proto/uv.lock' file

* chore: add 'otdf-python-proto/uv.lock' file

* fix: omit README from Github releases

* chore: document legacy version

* fix: address pre-commit (lint) issues

* chore: verbose output for pypi uploads

* fix: use correct 'extra-files' for uv.lock

See also: googleapis/release-please#2561

* chore: release 0.3.1

Release-As: 0.3.1

* chore: release develop (#82)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* chore: organize docs

* fix: remove unnecessary 'ncipollo/release-action'

* chore: add developer doc

* chore: CI improvements (#88)

* chore: prevent TestPyPI publishing <= 0.3.2

* chore: update .pre-commit-config.yaml

* chore: align versions

* chore: ensure future version alignment

* chore: comment unused GHA step

* chore: simplify version parsing

* chore: add tomli for Python < 3.11

* fix: get version dynamically in 'test_cli.py'

* fix: guarantee target-version decrypt support (#84)

* fix: add test data

* fix: improve target-version support

* fix: add get_cli_flags function

* fix: fix tests

* fix: bug handling bytes | BinaryIO & tests

* fix: update .gitignore

* fix: remove invalid default KAS

* fix: disable attrs for now

* fix: DRY test fixtures

* chore: cleanup

* fix:target mode encryption (#86)

* chore: update pre-commit

* fix: type annotations in tdf.py

* chore: expand inspect tests

* chore: cleanup tests

* chore: organize imports

* chore: require sorted imports

* chore: add test_cli_decrypt.py

* chore: organize integration tests

* chore: organize integration tests

* Tweak attributes

* chore: cleanup tests

* chore: cleanup tests

* chore: dry tests (#87)

* chore: dry tests

* chore: relocate run_cli_inspect

* chore: fix type annotation

* chore: note token isn't important

* chore: cleanup args & typing

* chore: extract 'get_platform_url' function

* chore: extract 'support_otdfctl_args' module

* chore: use '*get_cli_flags()' pattern

* chore: DRY code

* chore: DRY code

* chore: extract 'get_testing_environ' function

* chore: DRY code

* chore: DRY code

* chore: DRY code

* chore: improve pre-commit config

* fix: mirrored workflows for target-mode (#91)

* chore: cleanup for mirrored workflows

* chore: cleanup for mirrored workflows

* chore: cleanup for mirrored workflows

* chore: cleanup for mirrored workflows

* chore: cleanup for mirrored workflows

* chore: remove otdf-python-proto from manifest

* chore: cleanup and release (#93)

* fix: don't inspect without auth

* fix: process otdf-python-proto/pyproject.toml correctly

* chore: remove NanoTDF from README

* chore: mention legacy version in main README

* chore: set version to 0.3.1

* chore: fix release-please

* fix: release-please configuration (#95)

* fix: "jsonpath" in release-please-config.json

* chore: remove invalid changelog entries

* chore: cleanup branches used in release-please

* chore: remove invalid changelog file

* chore: reset version to 0.3.0

* chore: cleanup whitespace

* chore: improve release process

* chore: document release process

* chore: delete invalid information

* fix: update prerelease config for develop branch

* chore(develop): release otdf-python 0.3.1 (#96)

* chore(develop): release otdf-python 0.3.1

* Update CHANGELOG.md

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: b-long <[email protected]>

* fix: fix .release-please-config.json file (#97)

* fix: fix .release-please-config.json file

* chore: align for version 0.3.1

* chore: use importlib for version

* chore: manage .py files without relese-please

* fix: allow for development version in CLI version test

* Update src/otdf_python/cli.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* chore(develop): release otdf-python 0.3.2 (#98)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* fix: release configuration (#99)

* chore: fix release-please config

* chore: remove invalid changelog entries

* chore: roll back to 0.3.0

* fix: add develop-specific release-please files and update workflow

- Add .release-please-config-develop.json with prerelease: true
- Add .release-please-manifest-develop.json with current version
- Remove dynamic file creation from workflow
- Files are now committed to repo instead of generated at runtime

* chore(develop): release otdf-python 0.3.1 (#100)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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.

2 participants