-
Notifications
You must be signed in to change notification settings - Fork 13
Fix versioning #167
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
Fix versioning #167
Conversation
…ehavior for version display
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.
Pull Request Overview
This PR updates the version determination mechanism for the NSLS-II API, replacing the previous dynamic versioning via nsls2api._version with importlib.metadata and new dynamic versioning packages. The changes include:
- Replacing nsls2api._version with importlib.metadata for version retrieval in both CLI and API endpoints.
- Adjusting the Typer CLI configuration to support version printing without subcommands.
- Updating dependency and configuration settings in requirements.in and pyproject.toml to use uv-dynamic-versioning.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/nsls2api/cli/cli.py | Updated CLI version retrieval and Typer CLI configuration. |
| src/nsls2api/api/v1/stats_api.py | Updated API version retrieval logic and logging. |
| requirements.in | Added uv-dynamic-versioning dependency. |
| pyproject.toml | Updated project dynamic versioning settings and build backend. |
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.
Pull Request Overview
This PR fixes issues with dynamic versioning by switching the method of version determination to use the uv-dynamic-versioning package, and it addresses the CLI's handling of the --version flag.
- Updates version retrieval across the codebase
- Removes legacy import of nsls2api._version
- Adjusts CLI configuration to enable the version option without a subcommand
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/nsls2api/version.py | Implements a new get_version function using importlib.metadata |
| src/nsls2api/cli/cli.py | Updates CLI initialization and version retrieval logic |
| src/nsls2api/api/v1/stats_api.py | Switches to use get_version and logs API version |
| requirements.in | Adds uv-dynamic-versioning dependency |
| pyproject.toml | Configures uv-dynamic-versioning as the new version source |
padraic-shafer
left a comment
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.
The CLI command works as advertised.
$ uv run nsls2api --version
NSLS-II API CLI version: 0.2.2.dev663+a330bb4I raised a couple questions in the comments.
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.
Pull Request Overview
This PR replaces the previous dynamic versioning approach with the uv-dynamic-versioning package and updates both the CLI and API to use a centralized get_version function.
- Introduce
get_version()inversion.pyusingimportlib.metadata - Update CLI (
cli.py) to invoke version flag without subcommands and useget_version - Update API (
stats_api.py)/aboutendpoint to log and return version viaget_version - Add
uv-dynamic-versioningto dependencies and configure it inpyproject.toml
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/nsls2api/version.py | Added get_version() function for package version retrieval |
| src/nsls2api/cli/cli.py | Switched CLI to use get_version, enabled --version without subcommands |
| src/nsls2api/api/v1/stats_api.py | Updated /about endpoint to use get_version, removed old import |
| requirements.in | Added uv-dynamic-versioning to requirements |
| pyproject.toml | Configured uv-dynamic-versioning in build system, removed hatch-vcs |
Comments suppressed due to low confidence (1)
src/nsls2api/version.py:4
- Consider adding unit tests for 'get_version' to verify correct version retrieval when the package is installed and that it falls back to 'unknown' on PackageNotFoundError.
def get_version() -> str:
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The method I was using for dynamic versioning didn't seem to be working reliably. This PR changes the method to use a different package for version determination.
This also fixed an issue with the CLI where just running the command with no command and a flag would return an error (i.e. the
--versionwould always fail)To Test
If you checkout the branch, cd into it and then type
uv run nsls2api --versionshould return the version.Also, if you stand up the server, then the
/v1/aboutendpoint should also return the version.