Merge pephubclient into peppy#509
Merged
khoroshevskyi merged 138 commits intodev_claudefrom Dec 15, 2025
Merged
Conversation
* lint * Add tests, refactor
* Add working version of OAuth2 with github * Add tests * Add readme and constants * Update guthub client and tests
Co-authored-by: Vince <vince.reuter@gmail.com>
Release 0.4.4
2. added unwrapping for reg path
Release 0.4.5
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Transitional PR for typer CLI
There was a problem hiding this comment.
Pull request overview
This PR merges the pephubclient package into peppy and migrates the CLI from argparser to typer. The integration incorporates the full pephubclient codebase with git history, adds new pephubclient modules to the peppy package structure, and modernizes the CLI interface using typer for both the main peppy CLI and the eido subcommand.
Key Changes:
- Incorporated pephubclient code into
peppy.pephubclientmodule with OAuth, helpers, models, and client modules - Migrated CLI from argparse to typer for improved user experience and help documentation
- Removed deprecated
argparser.pyfile and replaced withcli.pyusing typer - Updated package configuration to include new pephubclient subpackages
Reviewed changes
Copilot reviewed 41 out of 45 changed files in this pull request and generated 36 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/phctests/test_pephubclient.py | Added comprehensive tests for PEPHubClient with parameterized error scenarios; contains typos and test logic issues |
| tests/phctests/test_manual.py | Manual integration tests for views and samples operations (marked as skipped) |
| tests/phctests/conftest.py | Test fixtures for JWT tokens and sample PEP data |
| peppy/pephubclient/pephubclient.py | Main PEPHubClient class implementing login, pull, push, and project operations; contains type and documentation issues |
| peppy/pephubclient/pephub_oauth/pephub_oauth.py | OAuth authentication flow for PEPhub device code authorization; contains spelling and documentation errors |
| peppy/pephubclient/helpers.py | Request management and message handling utilities; contains security issue with SSL verification disabled |
| peppy/pephubclient/modules/sample.py | Sample CRUD operations module for PEPhub projects |
| peppy/pephubclient/modules/view.py | View management module for PEPhub project views |
| peppy/cli.py | New typer-based CLI replacing argparse implementation |
| peppy/eido/cli.py | Migrated eido subcommands to typer |
| setup.py | Updated to include pephubclient subpackages in distribution |
| requirements/requirements-all.txt | Added pephubclient dependencies; contains leftover external dependency that should be removed |
| requirements/requirements-test.txt | Added test dependencies for pephubclient testing |
Comments suppressed due to low confidence (2)
tests/phctests/test_pephubclient.py:258
- Comparison of identical values; use cmath.isnan() if testing for not-a-number.
tests/phctests/test_pephubclient.py:453 - Comparison of identical values; use cmath.isnan() if testing for not-a-number.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if sample_name: | ||
| try: | ||
| sample_name = int(sample_name) | ||
| except ValueError: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
Suggested change
| except ValueError: | |
| except ValueError: | |
| # If sample_name is not an integer, keep it as a string (some sample names may be non-numeric) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
khoroshevskyi
approved these changes
Dec 15, 2025
Member
khoroshevskyi
left a comment
khoroshevskyi
left a comment
There was a problem hiding this comment.
LGTM
There is a lot of files, for now I will just merge everything, and do better review in next PR, where phc and eido are combinied
Comment on lines
+45
to
+48
| "peppy.eido", | ||
| "peppy.pephubclient", | ||
| "peppy.pephubclient.pephub_oauth", | ||
| "peppy.pephubclient.modules", |
Member
There was a problem hiding this comment.
I think you don't have to add here anything, it can be just package name
| importlib-metadata; python_version < '3.10' | ||
| jsonschema>=3.0.1 No newline at end of file | ||
| jsonschema>=3.0.1 | ||
| # pephubclient |
Member
There was a problem hiding this comment.
you can delete commented things
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Major changes: