NVIDIA / NVFlare

NVIDIA Federated Learning Application Runtime Environment
https://nvidia.github.io/NVFlare/
Apache License 2.0
592 stars 165 forks source link

BUGFIX: cloud deployment script issue with OSX #2677

Closed dirkpetersen closed 1 month ago

dirkpetersen commented 1 month ago

Sorry about this folks, should have known better but there was an OSX compatibility issue with my recent pull request 2650

This new PR addresses a bug with MacOS introduced with c73f6149a92d0b1c3d909381e33c4cf2e718e964 because "read -i" requires Bash >= 4 (newer than 2009 !) as OSX only comes with Bash 3.2 by default (you can use brew to install a newer version)

Refactor of prompt function has:

Types of changes

so if you are on Bash >= 4 you can edit the default values in place (except the AMI id)

* Cloud EC2 region, press ENTER to accept default: us-west-2
* Cloud AMI image name (use amd64 or arm64), press ENTER to accept default: ubuntu-*-22.04-amd64-pro-server
    retrieving AMI ID for ubuntu-*-22.04-amd64-pro-server ... ami-0812a6811f8b12a3f found
    finding smallest instance type with 1 GPUs and 15360 MiB VRAM ... g4dn.xlarge found
* Cloud AMI image, press ENTER to accept default [ami-0812a6811f8b12a3f]:
* Cloud EC2 type, press ENTER to accept default: g4dn.xlarge
region = us-west-2, ami image = ami-0812a6811f8b12a3f, EC2 type = g4dn.xlarge, OK? (Y/n):
If the client requires additional Python packages, please add them to:
    /home/dp/NVFlare/dirk/Test/AWS-T4.Y/startup/requirements.txt
Press ENTER when it's done or no additional dependencies. :

and if you are on older Bash, the default values will simply be shown in square brackets:

* Cloud EC2 region, press ENTER to accept default [us-west-2]:
* Cloud AMI image name (use amd64 or arm64), press ENTER to accept default [ubuntu-*-22.04-amd64-pro-server]:
    retrieving AMI ID for ubuntu-*-22.04-amd64-pro-server ... ami-0812a6811f8b12a3f found
    finding smallest instance type with 1 GPUs and 15360 MiB VRAM ... g4dn.xlarge found
* Cloud AMI image, press ENTER to accept default [ami-0812a6811f8b12a3f]:
* Cloud EC2 type, press ENTER to accept default [g4dn.xlarge]:
region = us-west-2, ami image = ami-0812a6811f8b12a3f, EC2 type = g4dn.xlarge, OK? (Y/n):
If the client requires additional Python packages, please add them to:
    /home/dp/NVFlare/dirk/Test/AWS-T4.Y/startup/requirements.txt
Press ENTER when it's done or no additional dependencies. :
dirkpetersen commented 1 month ago

@IsaacYangSLA if you could look at this quickly that would be great as PR 2650 breaks deployments from OSX

IsaacYangSLA commented 1 month ago

/build

IsaacYangSLA commented 1 month ago

Since this PR passed all tests, I will rebase and merge it without waiting for rebase from the PR owner.