Copilot commented on code in PR #13848:
URL: https://github.com/apache/trafficserver/pull/13848#discussion_r4293138850


##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -150,6 +150,15 @@ def configure_ats(obj: 'TestRun', server: 'Process', 
ats_config: dict, dns: Opti
         parent_line = parent_line.replace('{SERVER_HTTPS_PORT}', 
str(server.Variables.https_port))
         ts.Disk.parent_config.AddLine(parent_line)
 
+    # Configure strategies.yaml if specified.
+    strategies_yaml = ats_config.get('strategies_yaml')
+    if strategies_yaml is not None:
+        strategies_text = yaml.dump(strategies_yaml)
+        strategies_text = strategies_text.replace('{SERVER_HTTP_PORT}', 
str(server.Variables.http_port))
+        strategies_text = strategies_text.replace('{SERVER_HTTPS_PORT}', 
str(server.Variables.https_port))
+        ts.Disk.File(ts.Variables.CONFIGDIR + "/strategies.yaml", 
id="strategies_yaml", typename="ats:config")

Review Comment:
   Building a filesystem path via string concatenation is brittle across 
platforms/environments. Use `os.path.join(ts.Variables.CONFIGDIR, 
\"strategies.yaml\")` (or the project’s existing path helper if there is one) 
to keep path handling consistent.



##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -150,6 +150,15 @@ def configure_ats(obj: 'TestRun', server: 'Process', 
ats_config: dict, dns: Opti
         parent_line = parent_line.replace('{SERVER_HTTPS_PORT}', 
str(server.Variables.https_port))
         ts.Disk.parent_config.AddLine(parent_line)
 
+    # Configure strategies.yaml if specified.
+    strategies_yaml = ats_config.get('strategies_yaml')
+    if strategies_yaml is not None:
+        strategies_text = yaml.dump(strategies_yaml)
+        strategies_text = strategies_text.replace('{SERVER_HTTP_PORT}', 
str(server.Variables.http_port))
+        strategies_text = strategies_text.replace('{SERVER_HTTPS_PORT}', 
str(server.Variables.https_port))
+        ts.Disk.File(ts.Variables.CONFIGDIR + "/strategies.yaml", 
id="strategies_yaml", typename="ats:config")
+        ts.Disk.strategies_yaml.AddLines(strategies_text.split('\n'))

Review Comment:
   `yaml.dump()` can emit Python-specific tags and (depending on PyYAML 
version) may reorder keys, which can make generated configs less predictable. 
Prefer `yaml.safe_dump(...)` (and consider `sort_keys=False`) for stable, 
plain-YAML output. Also prefer `splitlines()` over `split('\\n')` to avoid 
adding a trailing empty line entry when the dumped text ends with a newline.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to