You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/05/21 10:31:06 UTC
[GitHub] [trafficserver] brbzull0 opened a new pull request #7866: Autest - Proxy Verifier Extension, add context template $-base string…
brbzull0 opened a new pull request #7866:
URL: https://github.com/apache/trafficserver/pull/7866
Add support for `$-base` string substitution on the passed replay file.
With this feature we now should be able to have a replay file with `$-base` strings which will be replaced by the PV extension using a context dict-like structure.
A good use case for this would be that a particular PV Server(b) needs to redirect to another PV Server(a) , so the port needs to be part of the host string in the server's(b) replay file:
`my_replaty_file.yaml`
```
....
server-response:
status: 302
reason: Redirect
headers:
fields:
- [ Location, "http://a.test:${a_port}/" ]
```
my unit test
```
...
Test.MakeVerifierServerProcess("b_server", "my_replaty_file.yaml",
context={"a_port": a_server.Variables.http_port})
```
--
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:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 commented on pull request #7866: Autest - Proxy Verifier Extension, add context template $-base string…
Posted by GitBox <gi...@apache.org>.
brbzull0 commented on pull request #7866:
URL: https://github.com/apache/trafficserver/pull/7866#issuecomment-847088990
[approve ci autest]
--
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:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a change in pull request #7866: Autest - Proxy Verifier Extension, add context template $-base string…
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7866:
URL: https://github.com/apache/trafficserver/pull/7866#discussion_r636930438
##########
File path: tests/gold_tests/autest-site/verifier_common.py
##########
@@ -33,3 +36,23 @@ def create_address_argument(ports):
argument += "127.0.0.1:{}".format(port)
argument += '"'
return argument
+
+# Perform substitution base on the passed context dict.
+# This function will return the new replay_path file
+
+
Review comment:
Comment blocks for functions in Python generally go inside the function. See `create_address_arguments` above. Thus:
```
def substitute_context_in_replay_file(process, replay_path, context):
"""
Perform substitution based upon the passed context dict.
This function will return the new replay_path file
"""
```
##########
File path: tests/gold_tests/autest-site/verifier_common.py
##########
@@ -33,3 +36,23 @@ def create_address_argument(ports):
argument += "127.0.0.1:{}".format(port)
argument += '"'
return argument
+
+# Perform substitution base on the passed context dict.
+# This function will return the new replay_path file
+
+
+def substitute_context_in_replay_file(process, replay_path, context):
+ # Only files for now
+ if os.path.isdir(replay_path):
+ raise ValueError(f"Mapping substitution not supported for directories.")
+
+ with open(os.path.join(process.TestDirectory, replay_path), 'r') as replay_file:
+ src = Template(replay_file.read())
+ result = src.substitute(context)
Review comment:
I suggest:
* src -> replay_template
* result -> replay_text
##########
File path: tests/gold_tests/autest-site/verifier_client.test.ext
##########
@@ -163,15 +169,27 @@ def AddVerifierClientProcess(run, name, replay_path, http_ports=None,
other_args: (str) Any other arbitrary options to pass to verifier-client.
+ context: Any dictionary-like object with keys that match the placeholders
+ in the replay file.
+ Template strings support $-based substitutions is supported in the
+ replay file.
+ You can refer to https://docs.python.org/3/library/string.html#template-strings
+ for more information how to add template strings to the replay file.
Returns:
The newly constructed verifier-client for the test run, which is also the
Default Process of the test run.
+ Throws:
+ ValueError:
+ If context substitution is expected to be done but a directory is passed as a
+ replay_path.
+ OSError in case of any issues related to I/O error, ie: File Not found for the replay
+ file when a context substitution is expected.
Review comment:
Will a `KeyError` be raised if `context` is missing variables in the replay template?
##########
File path: tests/gold_tests/autest-site/verifier_server.test.ext
##########
@@ -174,19 +180,29 @@ def MakeVerifierServerProcess(test, name, replay_path, http_ports=None,
other_args: (str) Any other arbitrary options to pass to verifier-server.
+ context: Any dictionary-like object with keys that match the placeholders
+ in the replay file.
+ Template strings support $-based substitutions is supported in the
+ replay file.
Review comment:
Looks like we want to remove: "is supported"
##########
File path: tests/gold_tests/autest-site/verifier_common.py
##########
@@ -33,3 +36,23 @@ def create_address_argument(ports):
argument += "127.0.0.1:{}".format(port)
argument += '"'
return argument
+
+# Perform substitution base on the passed context dict.
+# This function will return the new replay_path file
+
+
+def substitute_context_in_replay_file(process, replay_path, context):
+ # Only files for now
+ if os.path.isdir(replay_path):
+ raise ValueError(f"Mapping substitution not supported for directories.")
+
+ with open(os.path.join(process.TestDirectory, replay_path), 'r') as replay_file:
+ src = Template(replay_file.read())
+ result = src.substitute(context)
+ tf = tempfile.NamedTemporaryFile(delete=False, dir=process.RunDirectory, suffix=f"_{os.path.basename(replay_path)}")
+ with open(tf.name, "w") as new_replay_file:
+ new_replay_file.write(result)
+
+ replay_path = tf.name
Review comment:
If you move this before line 53, you can use it in the open.
##########
File path: tests/gold_tests/autest-site/verifier_client.test.ext
##########
@@ -163,15 +169,27 @@ def AddVerifierClientProcess(run, name, replay_path, http_ports=None,
other_args: (str) Any other arbitrary options to pass to verifier-client.
+ context: Any dictionary-like object with keys that match the placeholders
+ in the replay file.
+ Template strings support $-based substitutions is supported in the
+ replay file.
Review comment:
Looks like we want to remove: "is supported"
--
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:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 removed a comment on pull request #7866: Autest - Proxy Verifier Extension, add context template $-base string…
Posted by GitBox <gi...@apache.org>.
brbzull0 removed a comment on pull request #7866:
URL: https://github.com/apache/trafficserver/pull/7866#issuecomment-847088990
[approve ci autest]
--
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:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 commented on pull request #7866: Autest - Proxy Verifier Extension, add context template $-base string…
Posted by GitBox <gi...@apache.org>.
brbzull0 commented on pull request #7866:
URL: https://github.com/apache/trafficserver/pull/7866#issuecomment-847150516
[approve ci autest]
--
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:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a change in pull request #7866: Autest - Proxy Verifier Extension, add context template $-base string…
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7866:
URL: https://github.com/apache/trafficserver/pull/7866#discussion_r637061558
##########
File path: tests/gold_tests/autest-site/verifier_client.test.ext
##########
@@ -163,15 +169,27 @@ def AddVerifierClientProcess(run, name, replay_path, http_ports=None,
other_args: (str) Any other arbitrary options to pass to verifier-client.
+ context: Any dictionary-like object with keys that match the placeholders
+ in the replay file.
+ Template strings support $-based substitutions in the replay file.
+ You can refer to https://docs.python.org/3/library/string.html#template-strings
+ for more information how to add template strings to the replay file.
Returns:
The newly constructed verifier-client for the test run, which is also the
Default Process of the test run.
+ Throws:
+ ValueError:
+ If context substitution is expected to be done but a directory is passed as a
+ replay_path.
+ OSError in case of any issues related to I/O error, ie: File Not found for the replay
+ file when a context substitution is expected.
+ KeyError if placeholders are missing from the mapping between context and the replay
Review comment:
"replay" -> "replay file."
--
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:
users@infra.apache.org
[GitHub] [trafficserver] bneradt merged pull request #7866: Autest - Proxy Verifier Extension, add context template $-base string…
Posted by GitBox <gi...@apache.org>.
bneradt merged pull request #7866:
URL: https://github.com/apache/trafficserver/pull/7866
--
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:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #7866: Autest - Proxy Verifier Extension, add context template $-base string…
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7866:
URL: https://github.com/apache/trafficserver/pull/7866#issuecomment-893783781
Cherry-picked to v9.1.x branch.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org