-
Notifications
You must be signed in to change notification settings - Fork 0
chore: CI improvements #88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 focuses on enhancing the continuous integration and delivery pipeline by refining pre-commit hook configurations and streamlining the release management setup. These changes aim to improve the consistency of commit messages and simplify the automated versioning and release process for the project's Python packages.
Highlights
- Pre-commit Hook Configuration: The configuration for the
conventional-pre-commithook in.pre-commit-config.yamlhas been updated to correctly quote the list of scopes, ensuring proper parsing and validation of commit messages. - Release Please Configuration Streamlining: The
.release-please-config.jsonfile has been refactored to consolidate the release configuration forotdf-python-protointo the main package's configuration. This simplifies the release process by managing both packages under a singlerelease-pleaseentry and updates theextra-fileslist accordingly. - Version Bumps Across Packages: Multiple files related to package versions, including
pyproject.tomlfiles,__init__.pyfiles,uv.lockfiles, and.release-please-manifest.json, have been updated to reflect a new version0.3.2. This is a consequence of the CI improvements and release process.
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
-
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. ↩
There was a problem hiding this 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 includes improvements to the CI configuration, updating dependencies and version numbers across multiple files. The changes primarily focus on ensuring consistency in versioning and improving the reliability of the CI process. I have provided review comments addressing a potential issue with string formatting in .pre-commit-config.yaml.
|
/gemini review |
There was a problem hiding this 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 primarily focuses on CI improvements and version bumps. The changes to .pre-commit-config.yaml and .release-please-config.json are good steps towards better repository management. My review focuses on improving maintainability by removing hardcoded version numbers in the source code and tests. By dynamically reading the version from pyproject.toml, future releases will require fewer manual changes and be less error-prone.
| """ | ||
|
|
||
| __version__ = "0.3.1" | ||
| __version__ = "0.3.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoding the version number here can lead to it becoming out of sync with the version in pyproject.toml. It's better to fetch the version dynamically from the package metadata. This ensures it's always correct and reduces maintenance effort during releases.
| __version__ = "0.3.2" | |
| __version__ = __import__('importlib.metadata').version('otdf-python-proto') |
|
|
||
| # Version - get from project metadata | ||
| __version__ = "0.3.0" | ||
| __version__ = "0.3.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoding the version number here can lead to it becoming out of sync with the version in pyproject.toml. It's better to fetch the version dynamically from package metadata. This ensures it's always correct and reduces maintenance effort during releases.
| __version__ = "0.3.2" | |
| __version__ = __import__('importlib.metadata').version('otdf-python') |
|
/gemini review |
There was a problem hiding this 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 introduces several good CI improvements and dependency updates. Bumping the package version across multiple files and updating the lock files looks correct.
I've left a couple of suggestions for improving maintainability:
- In
src/otdf_python/cli.py, I've suggested dynamically loading the version frompyproject.tomlto avoid manual updates. - In
tests/test_cli.py, while making the version check dynamic is a great step, I've suggested a small refactoring to improve the code style and readability by moving the conditional imports to the top of the file.
Overall, these are solid improvements.
|
|
||
| # Version - get from project metadata | ||
| __version__ = "0.3.0" | ||
| __version__ = "0.3.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid manually updating the version string in multiple places, you can read it dynamically from the package metadata using importlib.metadata. This will ensure it's always in sync with pyproject.toml.
You'll need to add from importlib import metadata at the top of the file.
| __version__ = "0.3.2" | |
| __version__ = metadata.version("otdf-python") |
| with open(Path(__file__).parent.parent / "pyproject.toml", "rb") as f: | ||
| # Use tomli for Python < 3.11, tomllib for 3.11+ | ||
| if sys.version_info < (3, 11): | ||
| import tomli | ||
|
|
||
| pyproject = tomli.load(f) | ||
| else: | ||
| import tomllib | ||
|
|
||
| pyproject = tomllib.load(f) | ||
| expected_version = pyproject["project"]["version"] | ||
|
|
||
| assert expected_version in result.stdout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great improvement to make the test more robust!
To further improve readability and adhere to best practices (like PEP 8, which recommends imports at the top of the file), you can move the conditional import logic to the top of the file and simplify the TOML loading.
At the top of tests/test_cli.py, add:
import sys
from pathlib import Path
if sys.version_info < (3, 11):
import tomli as tomllib
else:
import tomllibThen you can simplify this part of the test.
with open(Path(__file__).parent.parent / "pyproject.toml", "rb") as f:
pyproject = tomllib.load(f)
expected_version = pyproject["project"]["version"]
assert expected_version in result.stdout* 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>
No description provided.