Skip to content

Conversation

@YoungHypo
Copy link
Contributor

@YoungHypo YoungHypo commented Apr 26, 2025

Do not merge. There are still bugs, and I try to fix it.

@YoungHypo YoungHypo requested a review from Copilot May 10, 2025 06:59
Copy link

Copilot AI left a 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 aims to upgrade the chaincode approve workflow by updating the API view, serializer, and underlying peer chaincode library.

  • Updated views to compute orderer_url and include an additional init_flag parameter.
  • Removed the redundant orderer_url field and added init_flag to the serializer.
  • Modified the lifecycle_approve_for_my_org method to adjust argument order and build the subprocess command dynamically.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/api-engine/api/routes/chaincode/views.py Adjusted the parameter order and removed the need to pass orderer_url from serializer.
src/api-engine/api/routes/chaincode/serializers.py Removed the orderer_url field and added the init_flag field for request validation.
src/api-engine/api/lib/peer/chaincode.py Changed the method signature and argument order in lifecycle_approve_for_my_org, and updated subprocess command construction.
Comments suppressed due to low confidence (2)

src/api-engine/api/lib/peer/chaincode.py:73

  • Using a list as the command with shell=True can lead to unexpected behavior. Consider setting shell=False or converting the command list into a single string.
res = subprocess.Popen(command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)

src/api-engine/api/lib/peer/chaincode.py:176

  • Using a list as the command with shell=True may not work as intended. It is recommended to either use shell=False with the list or join the list into a command string when shell=True is required.
res = subprocess.Popen(command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)

@yeasy yeasy marked this pull request as draft May 10, 2025 18:31
@YoungHypo YoungHypo force-pushed the issue-update-chaincode-query branch from c908ab3 to 6e8ada1 Compare May 17, 2025 04:20
@YoungHypo YoungHypo changed the title upgrade chaincode approve Upgrade Backend of Chaincode Approve May 17, 2025
@YoungHypo YoungHypo marked this pull request as ready for review May 17, 2025 04:31
@yeasy yeasy merged commit ae81505 into hyperledger-cello:main May 18, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants