Regenerate confidential ledger from typespec #41924
Conversation
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
| *, | ||
| collection_id: Optional[str] = None, | ||
| **kwargs: Any, | ||
| ) -> LROPoller[JSON]: | ||
| ) -> LROPoller[_models.TransactionStatus]: |
There was a problem hiding this comment.
If this signature hasnt been shipped as stable, it needs some attention, the true return type here should be LROPoller[LedgerEntry] however due to the underlying usage of the begin_wait_for_commit() method the poller seems to return transactionstatus instead
catalinaperalta
changed the title
This reverts commit 6351ead.
| __all__: List[str] = [ | ||
| "ConfidentialLedgerClientOperationsMixin" | ||
| ] # Add all objects you want publicly available to users at this package level | ||
| __all__: List[str] = [] # Add all objects you want publicly available to users at this package level |
There was a problem hiding this comment.
| __all__: List[str] = [] # Add all objects you want publicly available to users at this package level | |
| __all__: List[str] = ["_ConfidentialLedgerClientOperationsMixin"] # Add all objects you want publicly available to users at this package level |
One more suggestion: it is better put customized code before __all__
|
|
||
| __all__: List[str] = [ | ||
| "ConfidentialLedgerClientOperationsMixin" | ||
| ] # Add all objects you want publicly available to users at this package level |
There was a problem hiding this comment.
lmazuel
requested review from
scbedd,
mccoyp,
benbp,
weshaggard,
PallabPaul and
amruthashree18
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR regenerates the Azure Confidential Ledger SDK from the newly migrated TypeSpec project. The regeneration introduces significant changes to support TypeSpec-based code generation, including updated models, client configurations, and file formatting improvements.
Key changes:
- Migration from AutoRest to TypeSpec-based code generation
- Complete restructuring of the models and client architecture
- Introduction of new model classes and updated method signatures
- Comprehensive code formatting improvements across all test files and samples
Reviewed Changes
Copilot reviewed 82 out of 84 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tsp-location.yaml | New configuration file for TypeSpec-based SDK generation |
| pyproject.toml | Complete rebuild with new project configuration and dependencies |
| setup.py | Removed legacy setup.py file in favor of pyproject.toml |
| models/ | Complete regeneration of all model classes with TypeSpec architecture |
| certificate/ | Updated certificate client with new TypeSpec-generated structure |
| tests/ | Comprehensive code formatting and minor structural improvements |
| samples/ | Code formatting improvements and string concatenation fixes |
| receipt/ | Minor formatting improvements to receipt verification components |
| @@ -0,0 +1,1376 @@ | |||
| # pylint: disable=line-too-long,useless-suppression,too-many-lines | |||
There was a problem hiding this comment.
The pylint disable comment should be more specific. Consider disabling only the specific pylint rules that are necessary rather than using broad suppressions like 'useless-suppression'.
Copilot uses AI. Check for mistakes.
| @@ -607,17 +584,15 @@ def test_user_defined_role(self, confidentialledger_endpoint, confidentialledger | |||
|
|
|||
| role_name = "modify" | |||
|
|
|||
| client.create_user_defined_role([{"role_name": role_name, "role_actions": ["/content/read"]}]) | |||
| client.create_user_defined_role([Role({"role_name": role_name, "role_actions": ["/content/read"]})]) | |||
There was a problem hiding this comment.
The Role model is being instantiated with a dictionary. Consider using keyword arguments instead for better type safety: Role(role_name=role_name, role_actions=["/content/read"])
| client.create_user_defined_role([Role({"role_name": role_name, "role_actions": ["/content/read"]})]) | |
| client.create_user_defined_role([Role(role_name=role_name, role_actions=["/content/read"])]) |
Copilot uses AI. Check for mistakes.
| [ | ||
| {"role_name": role_name, "role_actions": ["/content/write", "/content/read"]} | ||
| ] | ||
| [Role({"role_name": role_name, "role_actions": ["/content/write", "/content/read"]})] |
There was a problem hiding this comment.
Same as above - consider using keyword arguments for Role instantiation: Role(role_name=role_name, role_actions=["/content/write", "/content/read"])
| [Role({"role_name": role_name, "role_actions": ["/content/write", "/content/read"]})] | |
| [Role(role_name=role_name, role_actions=["/content/write", "/content/read"])] |
Copilot uses AI. Check for mistakes.
Regenerate and update the Confidential Ledger library based on the newly migrated typespec project.