dasahcc commented on a change in pull request #647: Add java API to create 
cluster with CloudConfig
URL: https://github.com/apache/helix/pull/647#discussion_r363428608
 
 

 ##########
 File path: helix-core/src/main/java/org/apache/helix/model/CloudConfig.java
 ##########
 @@ -167,122 +136,108 @@ public String getCloudProvider() {
     return _record.getSimpleField(CloudConfigProperty.CLOUD_PROVIDER.name());
   }
 
+
   public static class Builder {
-    private String _clusterName = null;
-    private CloudProvider _cloudProvider;
-    private boolean _cloudEnabled = DEFAULT_CLOUD_ENABLED;
-    private String _cloudID;
-    private List<String> _cloudInfoSources;
-    private String _cloudInfoProcessorName;
+    private ZNRecord _record;
 
     public CloudConfig build() {
       validate();
-      return new CloudConfig(_clusterName, _cloudEnabled, _cloudProvider, 
_cloudID,
-          _cloudInfoSources, _cloudInfoProcessorName);
+      return new CloudConfig(_record);
     }
 
     /**
      * Default constructor
      */
     public Builder() {
+      _record = new ZNRecord(CLOUD_CONFIG_KW);
     }
 
     /**
-     * Constructor with Cluster Name as input
-     * @param clusterName
+     * Instantiate with a pre-populated record
+     * @param record a ZNRecord corresponding to a cloud configuration
      */
-    public Builder(String clusterName) {
-      _clusterName = clusterName;
+    public Builder(ZNRecord record) {
+      _record = record;
     }
 
     /**
      * Constructor with CloudConfig as input
      * @param cloudConfig
      */
     public Builder(CloudConfig cloudConfig) {
-      _cloudEnabled = cloudConfig.isCloudEnabled();
-      _cloudProvider = CloudProvider.valueOf(cloudConfig.getCloudProvider());
-      _cloudID = cloudConfig.getCloudID();
-      _cloudInfoSources = cloudConfig.getCloudInfoSources();
-      _cloudInfoProcessorName = cloudConfig.getCloudInfoProcessorName();
-    }
-
-    public Builder setClusterName(String v) {
-      _clusterName = v;
-      return this;
+      _record = cloudConfig.getRecord();
     }
 
     public Builder setCloudEnabled(boolean isEnabled) {
-      _cloudEnabled = isEnabled;
+      _record.setBooleanField(CloudConfigProperty.CLOUD_ENABLED.name(), 
isEnabled);
       return this;
     }
 
     public Builder setCloudProvider(CloudProvider cloudProvider) {
-      _cloudProvider = cloudProvider;
+      _record.setSimpleField(CloudConfigProperty.CLOUD_PROVIDER.name(), 
cloudProvider.name());
       return this;
     }
 
-    public Builder setCloudID(String v) {
-      _cloudID = v;
+    public Builder setCloudID(String cloudID) {
+      _record.setSimpleField(CloudConfigProperty.CLOUD_ID.name(), cloudID);
       return this;
     }
 
-    public Builder setCloudInfoSources(List<String> v) {
-      _cloudInfoSources = v;
+    public Builder setCloudInfoSources(List<String> cloudInfoSources) {
+      _record.setListField(CloudConfigProperty.CLOUD_INFO_SOURCE.name(), 
cloudInfoSources);
       return this;
     }
 
-    public Builder addCloudInfoSource(String v) {
-      if (_cloudInfoSources == null) {
-        _cloudInfoSources = new ArrayList<String>();
+    public Builder addCloudInfoSource(String cloudInfoSource) {
+      if (_record.getListField(CloudConfigProperty.CLOUD_INFO_SOURCE.name()) 
== null) {
+        _record.setListField(CloudConfigProperty.CLOUD_INFO_SOURCE.name(), new 
ArrayList<String>());
       }
-      _cloudInfoSources.add(v);
+      List<String> cloudInfoSourcesList = 
_record.getListField(CloudConfigProperty.CLOUD_INFO_SOURCE.name());
+      cloudInfoSourcesList.add(cloudInfoSource);
+      _record.setListField(CloudConfigProperty.CLOUD_INFO_SOURCE.name(), 
cloudInfoSourcesList);
       return this;
     }
 
-    public Builder setCloudInfoProcessorName(String v) {
-      _cloudInfoProcessorName = v;
+    public Builder setCloudInfoProcessorName(String cloudInfoProcessorName) {
+      
_record.setSimpleField(CloudConfigProperty.CLOUD_INFO_PROCESSOR_NAME.name(),
+          cloudInfoProcessorName);
       return this;
     }
 
-    public String getClusterName() {
-      return _clusterName;
-    }
-
-    public CloudProvider getCloudProvider() {
-      return _cloudProvider;
+    public String getCloudProvider() {
+      return _record.getSimpleField(CloudConfigProperty.CLOUD_PROVIDER.name());
     }
 
     public boolean getCloudEnabled() {
-      return _cloudEnabled;
+      return _record.getBooleanField(CloudConfigProperty.CLOUD_ENABLED.name(),
+          DEFAULT_CLOUD_ENABLED);
     }
 
     public String getCloudID() {
-      return _cloudID;
+      return _record.getSimpleField(CloudConfigProperty.CLOUD_ID.name());
     }
 
     public List<String> getCloudInfoSources() {
-      return _cloudInfoSources;
+      return 
_record.getListField(CloudConfigProperty.CLOUD_INFO_SOURCE.name());
     }
 
     public String getCloudInfoProcessorName() {
-      return _cloudInfoProcessorName;
+      return 
_record.getSimpleField(CloudConfigProperty.CLOUD_INFO_PROCESSOR_NAME.name());
     }
 
     private void validate() {
-      if (_cloudEnabled) {
-        if (_cloudID == null) {
-          throw new HelixException(
-              "This Cloud Configuration is Invalid. The CloudID is missing 
from the config.");
-        }
-        if (_cloudProvider == null) {
+      if (this.getCloudID() == null) {
 
 Review comment:
   Is "this" necessary?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to