phet commented on code in PR #4087:
URL: https://github.com/apache/gobblin/pull/4087#discussion_r1896450791


##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/dynamic/ScalingDirectiveParser.java:
##########
@@ -263,29 +283,50 @@ public static String asString(ScalingDirective directive) 
{
     directive.getOptDerivedFrom().ifPresent(derivedFrom -> {
       sb.append(',').append(derivedFrom.getBasisProfileName());
       sb.append(derivedFrom.getOverlay() instanceof ProfileOverlay.Adding ? 
"+(" : "-(");
-      ProfileOverlay overlay = derivedFrom.getOverlay();
-      if (overlay instanceof ProfileOverlay.Adding) {
-        ProfileOverlay.Adding adding = (ProfileOverlay.Adding) overlay;
-        for (ProfileOverlay.KVPair kv : adding.getAdditionPairs()) {
-          
sb.append(kv.getKey()).append('=').append(urlEncode(kv.getValue())).append(", 
");
-        }
-        if (adding.getAdditionPairs().size() > 0) {
-          sb.setLength(sb.length() - 2);  // remove trailing ", "
-        }
-      } else {
-        ProfileOverlay.Removing removing = (ProfileOverlay.Removing) overlay;
-        for (String key : removing.getRemovalKeys()) {
-          sb.append(key).append(", ");
-        }
-        if (removing.getRemovalKeys().size() > 0) {
-          sb.setLength(sb.length() - 2);  // remove trailing ", "
-        }
-      }
+      sb.append(stringifyProfileOverlay(derivedFrom.getOverlay()));
       sb.append(')');
     });
     return sb.toString();
   }
 
+  /** @return the `scalingDirective` invariably stringified as two parts, a 
{@link StringWithPlaceholderPlusOverlay} - regardless of stringified length */
+  public static StringWithPlaceholderPlusOverlay 
asStringWithPlaceholderPlusOverlay(ScalingDirective directive) {
+    StringBuilder sb = new StringBuilder();
+    
sb.append(directive.getTimestampEpochMillis()).append('.').append(directive.getProfileName()).append('=').append(directive.getSetPoint());
+    Optional<String> optProfileOverlayStr = 
directive.getOptDerivedFrom().map(derivedFrom ->
+        stringifyProfileOverlay(derivedFrom.getOverlay())
+    );
+    directive.getOptDerivedFrom().ifPresent(derivedFrom -> {
+      sb.append(',').append(derivedFrom.getBasisProfileName());
+      sb.append(derivedFrom.getOverlay() instanceof ProfileOverlay.Adding ? 
"+(" : "-(");
+      sb.append(OVERLAY_DEFINITION_PLACEHOLDER);
+      sb.append(')');
+    });
+    return new StringWithPlaceholderPlusOverlay(sb.toString(), 
optProfileOverlayStr.orElse(""));
+  }
+
+  private static String stringifyProfileOverlay(ProfileOverlay overlay) {
+    StringBuilder sb = new StringBuilder();
+    if (overlay instanceof ProfileOverlay.Adding) {
+      ProfileOverlay.Adding adding = (ProfileOverlay.Adding) overlay;
+      for (ProfileOverlay.KVPair kv : adding.getAdditionPairs()) {
+        
sb.append(kv.getKey()).append('=').append(urlEncode(kv.getValue())).append(", 
");
+      }
+      if (adding.getAdditionPairs().size() > 0) {
+        sb.setLength(sb.length() - 2);  // remove trailing ", "
+      }
+    } else {
+      ProfileOverlay.Removing removing = (ProfileOverlay.Removing) overlay;
+      for (String key : removing.getRemovalKeys()) {
+        sb.append(key).append(", ");
+      }
+      if (removing.getRemovalKeys().size() > 0) {
+        sb.setLength(sb.length() - 2);  // remove trailing ", "
+      }
+    }
+    return sb.toString();
+  }

Review Comment:
   it could, but truly this is the inverse of `ScalingDirectiveParser::parse`, 
so it feels appropriate to live together in this class.  after all, I'm not 
ready define `asString(ScalingDirective)` as a `ScalingDirective` method.
   
   usually, the particulars of a format should be separate from the object of 
formatting, to allow multiple formats for the same type.
   
   but you did help flag that I forgot handling of `ProfileOverlay.Combo`, so I 
now throw `IllegalArgumentException`



-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to