curveadm icon indicating copy to clipboard operation
curveadm copied to clipboard

Updated hostname to hostIp and updated methods and configs accordingly.

Open Jason011125 opened this issue 1 year ago • 3 comments

#260 Improved hostname by changing it to hostIp, which represents its true usage.

Jason011125 avatar Aug 15 '23 05:08 Jason011125

Hi @Jason011125 Your commit is good, but we also need to consider the issue of backward compatibility. The existing cluster yaml configuration file writes the hostname field. After upgrading to the curveadm version that includes your pr, an error will be reported when performing related operations. Therefore, we need to ensure the compatibility of the hostname configuration, and it is recommended to use the hostip field in the configuration file of the new version, and discard the hostname field after several versions.

Can you continue to improve this PR?

aspirer avatar Aug 15 '23 07:08 aspirer

Hi @Jason011125 Your commit is good, but we also need to consider the issue of backward compatibility. The existing cluster yaml configuration file writes the hostname field. After upgrading to the curveadm version that includes your pr, an error will be reported when performing related operations. Therefore, we need to ensure the compatibility of the hostname configuration, and it is recommended to use the hostip field in the configuration file of the new version, and discard the hostname field after several versions.

Can you continue to improve this PR?

Agree! On the other hand, i think use name to replace host and reserve hostname is a good choise, e.g.

hosts:
  - name: server1
    hostname: 10.0.1.1

OR

hosts:
  - name: server1
    hostname: server1.163.org

Wine93 avatar Aug 20 '23 03:08 Wine93

Hey guys, can we take another look at the PR as I amend the hostip part to allow upward compatability.

Jason011125 avatar Aug 20 '23 08:08 Jason011125