feat: Refactor custom_fee_royalty.py example for end-to-end execution #1155
Conversation
|
Warning Rate limit exceeded@Adityarya11 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR transforms the Changes
Sequence DiagramsequenceDiagram
actor User
participant Script as custom_fee_royalty.py
participant Client
participant Network as Hedera Network
participant Ledger
User->>Script: uv run example
Script->>Script: load_dotenv()
Script->>Client: setup_client() with OPERATOR_ID/KEY
Client->>Network: Initialize and validate connection
Network-->>Client: Connection established
Script->>Script: Build CustomFixedFee (1 HBAR fallback)
Script->>Script: Build CustomRoyaltyFee (5%)
Script->>Script: Create TokenCreateTransaction with fees
Script->>Network: Submit TokenCreateTransaction (sign & execute)
Network->>Ledger: Create non-fungible token with fees
Ledger-->>Network: Receipt with token ID
Network-->>Script: Transaction receipt
Script->>Script: Check receipt status
Script->>Network: TokenInfoQuery to verify fees
Network->>Ledger: Query token information
Ledger-->>Network: Token details with fees
Network-->>Script: Fee configuration confirmed
Script->>User: Print token ID and verified fees
Script->>Client: Close client connection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1155 +/- ##
=======================================
Coverage 91.19% 91.19%
=======================================
Files 139 139
Lines 8447 8447
=======================================
Hits 7703 7703
Misses 744 744 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
CHANGELOG.md (1)
81-81: Consider adding issue/PR reference for consistency.Many entries in this changelog include issue or PR references (e.g.,
(#1154)). Adding the reference would improve traceability.-- Transformed `examples/tokens/custom_fee_royalty.py` to be an end-to-end example, that interacts with the Hedera network, rather than a static object demo. +- Transformed `examples/tokens/custom_fee_royalty.py` to be an end-to-end example, that interacts with the Hedera network, rather than a static object demo. (#1154)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)examples/tokens/custom_fee_royalty.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.8)
examples/tokens/custom_fee_royalty.py
41-41: Abstract raise to an inner function
(TRY301)
41-41: Avoid specifying long messages outside the exception class
(TRY003)
48-48: Consider moving this statement to an else block
(TRY300)
78-78: f-string without any placeholders
Remove extraneous f prefix
(F541)
79-79: f-string without any placeholders
Remove extraneous f prefix
(F541)
81-81: f-string without any placeholders
Remove extraneous f prefix
(F541)
121-121: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Agent
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: run-examples
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.11)
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (3)
examples/tokens/custom_fee_royalty.py (3)
7-25: LGTM!Imports and environment configuration follow the established pattern from other examples in this repository.
82-97: LGTM!Token creation is correctly configured as a non-fungible token with royalty fees, following the requirements from issue #1154.
99-128: LGTM!The execution flow with try/except/finally ensures proper cleanup, and the token info query correctly verifies the custom fees. The broad
Exceptioncatch (line 121) is acceptable for a demonstration example to capture any network or SDK errors gracefully.
| print(f"Royalty Fee Configured: 5/100 (5%)") | ||
| print(f"Fallback Fee: 1 HBAR") | ||
|
|
||
| print(f"\n--- Creating TOken with Royalty Fee ---") |
There was a problem hiding this comment.
Remove extraneous f prefixes from strings without placeholders.
Lines 78, 79, and 81 use f-strings but contain no placeholders. Also, line 81 has a typo: "TOken" → "Token".
🔎 Proposed fix
- print(f"Royalty Fee Configured: 5/100 (5%)")
- print(f"Fallback Fee: 1 HBAR")
+ print("Royalty Fee Configured: 5/100 (5%)")
+ print("Fallback Fee: 1 HBAR")
- print(f"\n--- Creating TOken with Royalty Fee ---")
+ print("\n--- Creating Token with Royalty Fee ---")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| print(f"Royalty Fee Configured: 5/100 (5%)") | |
| print(f"Fallback Fee: 1 HBAR") | |
| print(f"\n--- Creating TOken with Royalty Fee ---") | |
| print("Royalty Fee Configured: 5/100 (5%)") | |
| print("Fallback Fee: 1 HBAR") | |
| print("\n--- Creating Token with Royalty Fee ---") |
🧰 Tools
🪛 Ruff (0.14.8)
78-78: f-string without any placeholders
Remove extraneous f prefix
(F541)
79-79: f-string without any placeholders
Remove extraneous f prefix
(F541)
81-81: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In examples/tokens/custom_fee_royalty.py around lines 78 to 81, the three print
statements use unnecessary f-string prefixes and one contains a typo ("TOken"
should be "Token"); remove the leading f from those string literals (use normal
strings) and correct "TOken" to "Token" so the lines read as plain prints
without f-strings and with the fixed capitalization/word.
There was a problem hiding this comment.
This is a reasonable suggestion.
f strings should only be used if you are adding some variable inside it e.g.
print(f"Fallback Fee: {Amount} HBAR")
otherwise you can just do
print("Fallback Fee: 1 HBAR")
The bigger issue is you should pull these variables in from what you added to the token, then add them to your f string
There was a problem hiding this comment.
@exploreriii, you're absolutely right! Pulling the values directly from the fee objects makes the code more robust and maintainable. Here's how the print statements should be refactored to use actual values from the objects:
# Extract values from the fee objects
fallback_amount_hbar = Hbar.from_tinybars(fallback_fee.amount)
royalty_numerator = royalty_fee.numerator
royalty_denominator = royalty_fee.denominator
royalty_percentage = (royalty_numerator / royalty_denominator) * 100
print(f"Royalty Fee Configured: {royalty_numerator}/{royalty_denominator} ({royalty_percentage}%)")
print(f"Fallback Fee: {fallback_amount_hbar} HBAR")
print("\n--- Creating Token with Royalty Fee ---")This way:
- If someone changes
numerator=5tonumerator=10, the print will automatically show "10/100 (10%)" - If the fallback fee changes from
Hbar(1)toHbar(2), it will automatically display "2 HBAR" - The f-strings now have a clear purpose - interpolating actual variables
There was a problem hiding this comment.
Pull request overview
This PR refactors the examples/tokens/custom_fee_royalty.py example to be a full end-to-end executable demonstration that interacts with the Hedera network, replacing the previous static object creation example. The update aligns this example with similar token examples in the repository.
- Transforms a static demo into an interactive network example with actual token creation and verification
- Adds comprehensive setup, execution, and verification workflow
- Updates CHANGELOG.md to document the enhancement
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| examples/tokens/custom_fee_royalty.py | Complete refactor from static object demonstration to end-to-end network example with client setup, token creation with custom royalty fee, and network verification |
| CHANGELOG.md | Documents the transformation of custom_fee_royalty.py to an end-to-end example |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if receipt.status != ResponseCode.SUCCESS: | ||
| print(f"Transaction failed with status: {ResponseCode(receipt.status).name}") | ||
| return |
There was a problem hiding this comment.
Missing early return after error status check. When the transaction fails (line 103-105), the function should return early to prevent executing the remaining code. Without this return, the code will continue to line 107 and attempt to access receipt.token_id, which may not be set when the transaction fails, potentially causing an error.
| """ | ||
| Run with: | ||
| uv run examples/tokens/custom_royalty_fee.py | ||
| python examples/tokens/custom_royalty_fee.py |
There was a problem hiding this comment.
The filename in the comment is incorrect. It should be 'custom_fee_royalty.py' to match the actual filename, not 'custom_royalty_fee.py'.
There was a problem hiding this comment.
your file name should be custom_royalty_fee.py to match the exact src name
src/hiero_sdk_python/tokens/custom_royalty_fee.py
Signed-off-by: Adityarya11 <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: notsogod <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: notsogod <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: notsogod <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: notsogod <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: notsogod <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: notsogod <[email protected]>
Adityarya11
force-pushed
the
feat/e2e-custom-fee-royalty#1154
branch
from
e63921e to
bc0f378
Compare
|
[commit-verification-bot] To achieve verified status, please read: Remember, you require a GPG key and each commit must be signed with: Thank you for contributing! From the Hiero Python SDK Team |
exploreriii
left a comment
exploreriii
left a comment
There was a problem hiding this comment.
Hi some of your commit are co authored by copilot, but i don't think its set up correctly with your GPG key as all your commits are not signed correctly
https://github.com/hiero-ledger/hiero-sdk-python/pull/1155/commits
would suggest soft reverting + starting fresh
|
hi @exploreriii |
2 participants
This pull request updates the
examples/tokens/custom_fee_royalty.pyexample to be a full end-to-end demonstration that interacts with the Hedera network, rather than just showing static object creation. It also updates the changelog to reflect this enhancement.End-to-end example improvements:
examples/tokens/custom_fee_royalty.pyto connect to the Hedera network, create a token with a custom royalty fee, and verify the fee on the network, making it a practical, executable example.Documentation updates:
CHANGELOG.mdto note thatexamples/tokens/custom_fee_royalty.pyis now an end-to-end network example.Description:Related issue(s):
Fixes #1154
Notes for reviewer:
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.