Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test-network-k8s: Improve prereqs logic #1241

Merged
merged 1 commit into from
Aug 21, 2024
Merged

test-network-k8s: Improve prereqs logic #1241

merged 1 commit into from
Aug 21, 2024

Conversation

satota2
Copy link
Contributor

@satota2 satota2 commented Jul 31, 2024

It seems that PR #1205 has stalled, so I have created a new PR that builds upon the considerations from that one while addressing the issues raised.

This PR resolves issue #1204 and also addresses the concerns mentioned in PR #1205. Specifically, it:

  • Uses the newer install script instead of bootstrap.sh.
  • Downloads binaries corresponding to the image versions, supporting both two-digit and three-digit Fabric version specifications.

Related issue:
Resolves #1204

@satota2 satota2 requested a review from a team as a code owner July 31, 2024 09:42
bestbeforetoday
bestbeforetoday approved these changes Aug 8, 2024
View reviewed changes
Copy link
Member

@bestbeforetoday bestbeforetoday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same sed expression to extract a version number is repeated several times. Perhaps for ease of maintenance this could be extracted to a function so there is only one place to change it in the future? Otherwise this looks like a good change to me

@satota2
test-network-k8s: Improve prereqs logic
c4daea3 
This patch improves prereqs logic in test-network-k8s.
- Use the newer install script instead of bootstrap.sh
- Download binaries matching the Docker image versions, instead of the latest version
- Add checks for Fabric versions to ensure consistency between images and binaries

Signed-off-by: Tatsuya Sato <tatsuya.sato.so@hitachi.com>
@satota2 satota2 force-pushed the improve-prereqs-test-net-k8s branch from 22ff079 to c4daea3 Compare August 19, 2024 07:35
@satota2
Copy link
Contributor Author

satota2 commented Aug 19, 2024

The same sed expression to extract a version number is repeated several times. Perhaps for ease of maintenance this could be extracted to a function so there is only one place to change it in the future? Otherwise this looks like a good change to me

@bestbeforetoday
Apologies for the delayed response; I was on summer vacation in Japan.

Thank you for your review. You are absolutely right. I've updated the code to extract the regular expression into a variable for easier maintenance. Please take a look when you have a moment.

bestbeforetoday
bestbeforetoday approved these changes Aug 19, 2024
View reviewed changes
Copy link
Member

@bestbeforetoday bestbeforetoday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

This was referenced Aug 21, 2024
Closed
Closed
@denyeart denyeart merged commit ce5aa88 into hyperledger:main Aug 21, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@denyeart denyeart denyeart approved these changes

@bestbeforetoday bestbeforetoday bestbeforetoday approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test-network-k8s network script gets last version Fabric bin files instead of requested version
3 participants
@satota2 @bestbeforetoday @denyeart