chore: add native client to tests #163
Draft
Conversation
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
jcabrero
force-pushed
the
chore/add_native_client_to_tests
branch
from
d4a3527 to
4f69632
Compare
jcabrero
changed the title
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a new Python SDK client (nilai-py) for the Nilai platform and updates several dependencies to newer versions. The main changes include:
- Adding the
nilai-pyclient package with delegation token management and OpenAI-compatible functionality - Upgrading dependencies:
blindfold(0.1.0rc0 → 0.1.0),isort(6.1.0 → 7.0.0),jiter(0.9.0 → 0.11.0),openai(1.70.0 → 2.4.0),pydantic(2.11.10 → 2.12.2),pydantic-core(2.33.2 → 2.41.4),pytest(8.3.5 → 8.4.2),ruff(0.11.7 → 0.14.1), and others - Adding
coverage(7.11.0) andpytest-cov(7.0.0) for test coverage - Updating
secretvaultsdependency to use a different repository and branch - Refactoring configuration modules to separate concerns
- Removing deprecated logger utilities and updating imports
Reviewed Changes
Copilot reviewed 56 out of 62 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Updated lock file with new dependencies and version upgrades |
| tests/e2e/test_openai.py | Refactored NUC token handling and removed test cases |
| tests/e2e/test_http.py | Updated token handling and removed test cases |
| tests/e2e/nuc.py | Complete rewrite using new nilai-py client |
| tests/e2e/config.py | Updated to use new client and auth token |
| tests/conftest.py | Added pytest configuration for StreamWriter error suppression |
| scripts/credit-init.sql | Updated test credentials with new DID format |
| pyproject.toml | Added nilai-py dependency and updated isort version |
| packages/nilai-common/... | Refactored config modules and removed logger |
| nilai-models/... | Added vLLM template and updated daemon config access |
| nilai-api/... | Updated imports and added metering bypass for docs token |
| docker/... | Updated vLLM template paths |
| clients/nilai-py/... | New SDK client package with comprehensive functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ENVIRONMENT = CONFIG.environment.environment | ||
| # Left for API key for backwards compatibility | ||
| AUTH_TOKEN = CONFIG.auth.auth_token | ||
| AUTH_TOKEN = "SecretTestApiKey" |
There was a problem hiding this comment.
Hardcoded test API key should not be committed. Consider using environment variables or a secure configuration management system to store test credentials.
|
|
||
|
|
||
| # Global host settings instance | ||
| SETTINGS: HostSettings = HostSettings( |
There was a problem hiding this comment.
The global variable 'SETTINGS' is not used.
|
|
||
|
|
||
| # Global model settings instance | ||
| MODEL_SETTINGS: ModelSettings = ModelSettings( |
There was a problem hiding this comment.
The global variable 'MODEL_SETTINGS' is not used.
| ) | ||
|
|
||
| # Global model capabilities instance | ||
| MODEL_CAPABILITIES: ModelCapabilities = ModelCapabilities( |
There was a problem hiding this comment.
The global variable 'MODEL_CAPABILITIES' is not used.
|
|
||
| if __name__ == "__main__": | ||
| try: | ||
| version = main() |
There was a problem hiding this comment.
The global variable 'version' is not used.
Suggested change
| version = main() | |
| main() |
| delegation_token_response.delegation_token | ||
| ) | ||
|
|
||
| def _get_invocation_token(self) -> str: |
There was a problem hiding this comment.
Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.
Comment on lines
+53
to
+55
| print( | ||
| f"Using NilDB Configuration:\n Nilchain URL: {DefaultNilDBConfig.nilchain_url}\n Nilauth URL: {DefaultNilDBConfig.nilauth_url}\n Nodes: {DefaultNilDBConfig.nodes}\n Collection ID: {DefaultNilDBConfig.collection}" | ||
| ) |
There was a problem hiding this comment.
Print statement may execute during import.
Suggested change
| print( | |
| f"Using NilDB Configuration:\n Nilchain URL: {DefaultNilDBConfig.nilchain_url}\n Nilauth URL: {DefaultNilDBConfig.nilauth_url}\n Nodes: {DefaultNilDBConfig.nodes}\n Collection ID: {DefaultNilDBConfig.collection}" | |
| ) | |
| if __name__ == "__main__": | |
| print( | |
| f"Using NilDB Configuration:\n Nilchain URL: {DefaultNilDBConfig.nilchain_url}\n Nilauth URL: {DefaultNilDBConfig.nilauth_url}\n Nodes: {DefaultNilDBConfig.nodes}\n Collection ID: {DefaultNilDBConfig.collection}" | |
| ) |
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.
No description provided.